From 821a7911ade057aa2be5971c657c1e0f453398e7 Mon Sep 17 00:00:00 2001 From: Wolf480pl Date: Mon, 11 Aug 2014 11:33:36 +0200 Subject: [PATCH 1/3] Started prototyping the artifact loading system Where "artifact" means something that contains code, eg. a jar, a maven artifact, a folder with classes. I'm starting with state tracking, later I'll add actual means of loading the code from artifacts. --- .../plugins/artifact/Artifact.java | 25 +++++++ .../plugins/artifact/ArtifactJob.java | 10 +++ .../plugins/artifact/ArtifactManager.java | 72 +++++++++++++++++++ .../plugins/artifact/ArtifactState.java | 5 ++ .../plugins/artifact/jobs/AbstractJob.java | 15 ++++ .../plugins/artifact/jobs/LocateJob.java | 14 ++++ .../plugins/artifact/jobs/RemoveJob.java | 13 ++++ 7 files changed, 154 insertions(+) create mode 100644 src/main/java/com/flowpowered/plugins/artifact/Artifact.java create mode 100644 src/main/java/com/flowpowered/plugins/artifact/ArtifactJob.java create mode 100644 src/main/java/com/flowpowered/plugins/artifact/ArtifactManager.java create mode 100644 src/main/java/com/flowpowered/plugins/artifact/ArtifactState.java create mode 100644 src/main/java/com/flowpowered/plugins/artifact/jobs/AbstractJob.java create mode 100644 src/main/java/com/flowpowered/plugins/artifact/jobs/LocateJob.java create mode 100644 src/main/java/com/flowpowered/plugins/artifact/jobs/RemoveJob.java diff --git a/src/main/java/com/flowpowered/plugins/artifact/Artifact.java b/src/main/java/com/flowpowered/plugins/artifact/Artifact.java new file mode 100644 index 0000000..0f62820 --- /dev/null +++ b/src/main/java/com/flowpowered/plugins/artifact/Artifact.java @@ -0,0 +1,25 @@ +package com.flowpowered.plugins.artifact; + +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; + +public class Artifact { + private Queue jobQueue = new ConcurrentLinkedQueue<>(); + private ArtifactState state = ArtifactState.UNDEFINED; + + public ArtifactState getState() { + return state; + } + + /** + * Should be only called from the thread that is currently executing {@link ArtifactManager#pulse(String)} + */ + public void setState(ArtifactState state) { + this.state = state; + } + + public Queue getJobQueue() { + return jobQueue; + } + +} diff --git a/src/main/java/com/flowpowered/plugins/artifact/ArtifactJob.java b/src/main/java/com/flowpowered/plugins/artifact/ArtifactJob.java new file mode 100644 index 0000000..6deeebc --- /dev/null +++ b/src/main/java/com/flowpowered/plugins/artifact/ArtifactJob.java @@ -0,0 +1,10 @@ +package com.flowpowered.plugins.artifact; + +import com.flowpowered.commons.SimpleFuture; + +public interface ArtifactJob { + + SimpleFuture getFuture(); + + void run(Artifact artifact); +} diff --git a/src/main/java/com/flowpowered/plugins/artifact/ArtifactManager.java b/src/main/java/com/flowpowered/plugins/artifact/ArtifactManager.java new file mode 100644 index 0000000..bfaf54a --- /dev/null +++ b/src/main/java/com/flowpowered/plugins/artifact/ArtifactManager.java @@ -0,0 +1,72 @@ +package com.flowpowered.plugins.artifact; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.Future; + +import com.flowpowered.plugins.artifact.jobs.LocateJob; +import com.flowpowered.plugins.artifact.jobs.RemoveJob; + +public class ArtifactManager { + private ConcurrentMap byName = new ConcurrentHashMap<>(); + + + /** + * Makes the Manager try to find the artifact and start tracking it. + * @return some Future, whose type will be specified once I figure out what I want it to be + */ + public Future locate(String artifactName) { + LocateJob job = new LocateJob(); + + Artifact newArtifact = new Artifact(); + newArtifact.getJobQueue().add(job); + + Artifact artifact = byName.putIfAbsent(artifactName, newArtifact); + if (artifact == null) { + enqueuePulse(artifactName); + } else { + // FIXME: Race condition - we may enqueue the job too late. + artifact.getJobQueue().add(job); + } + return job.getFuture(); + } + + /** + * In any given moment this method can be running at most once per artifact + */ + public void pulse(String artifactName) { + Artifact artifact = byName.get(artifactName); + if (artifact == null) { + // Should not happen, only we're allowed to set it to null and wouldn't pulse after that + throw new IllegalStateException("pulsed on nonexistent artifact"); + } + ArtifactJob job = artifact.getJobQueue().poll(); + if (job != null) { + // TODO: Job merging + job.run(artifact); + + if (job instanceof RemoveJob) { + byName.remove(artifactName); + + for (ArtifactJob j : artifact.getJobQueue()) { + if (j instanceof LocateJob) { + locate(artifactName); + // TODO: add the rest of the queue to the new artifact? + break; + } + } + + return; // Don't requeue ourselves; + } + } + enqueuePulse(artifactName); + } + + /** + * Makes some thread call {@link #pulse(String)} for the given artifact soon. + * The definition of "soon" is implementation-specific. + */ + protected void enqueuePulse(String artifactName) { + + } +} diff --git a/src/main/java/com/flowpowered/plugins/artifact/ArtifactState.java b/src/main/java/com/flowpowered/plugins/artifact/ArtifactState.java new file mode 100644 index 0000000..cd123a4 --- /dev/null +++ b/src/main/java/com/flowpowered/plugins/artifact/ArtifactState.java @@ -0,0 +1,5 @@ +package com.flowpowered.plugins.artifact; + +public enum ArtifactState { + UNDEFINED, LOCATED, LOADING, LOADED, WAITING_FOR_DEPS, MISSING_DEPS, RESOLVED, UNLOADING, UNLOADED +} diff --git a/src/main/java/com/flowpowered/plugins/artifact/jobs/AbstractJob.java b/src/main/java/com/flowpowered/plugins/artifact/jobs/AbstractJob.java new file mode 100644 index 0000000..4d03988 --- /dev/null +++ b/src/main/java/com/flowpowered/plugins/artifact/jobs/AbstractJob.java @@ -0,0 +1,15 @@ +package com.flowpowered.plugins.artifact.jobs; + +import com.flowpowered.commons.SimpleFuture; + +import com.flowpowered.plugins.artifact.ArtifactJob; + +public abstract class AbstractJob implements ArtifactJob { + protected SimpleFuture future = new SimpleFuture<>(); + + @Override + public SimpleFuture getFuture() { + return future; + } + +} \ No newline at end of file diff --git a/src/main/java/com/flowpowered/plugins/artifact/jobs/LocateJob.java b/src/main/java/com/flowpowered/plugins/artifact/jobs/LocateJob.java new file mode 100644 index 0000000..a0e80ad --- /dev/null +++ b/src/main/java/com/flowpowered/plugins/artifact/jobs/LocateJob.java @@ -0,0 +1,14 @@ +package com.flowpowered.plugins.artifact.jobs; + +import com.flowpowered.plugins.artifact.Artifact; +import com.flowpowered.plugins.artifact.ArtifactState; + +public class LocateJob extends AbstractJob { + @Override + public void run(Artifact artifact) { + // TODO Auto-generated method stub + artifact.setState(ArtifactState.LOCATED); + future.setResult(null); + } + +} diff --git a/src/main/java/com/flowpowered/plugins/artifact/jobs/RemoveJob.java b/src/main/java/com/flowpowered/plugins/artifact/jobs/RemoveJob.java new file mode 100644 index 0000000..9032474 --- /dev/null +++ b/src/main/java/com/flowpowered/plugins/artifact/jobs/RemoveJob.java @@ -0,0 +1,13 @@ +package com.flowpowered.plugins.artifact.jobs; + +import com.flowpowered.plugins.artifact.Artifact; + +public class RemoveJob extends AbstractJob { + + @Override + public void run(Artifact artifact) { + // TODO Auto-generated method stub + future.setResult(null); + } + +} From 465b3281aab0753214f912864a83be840f9f7220 Mon Sep 17 00:00:00 2001 From: Wolf480pl Date: Tue, 12 Aug 2014 12:04:04 +0200 Subject: [PATCH 2/3] Fix the race condition --- .../java/com/flowpowered/plugins/artifact/Artifact.java | 8 ++++++++ .../com/flowpowered/plugins/artifact/ArtifactManager.java | 8 +++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/flowpowered/plugins/artifact/Artifact.java b/src/main/java/com/flowpowered/plugins/artifact/Artifact.java index 0f62820..31b6ed9 100644 --- a/src/main/java/com/flowpowered/plugins/artifact/Artifact.java +++ b/src/main/java/com/flowpowered/plugins/artifact/Artifact.java @@ -6,6 +6,7 @@ public class Artifact { private Queue jobQueue = new ConcurrentLinkedQueue<>(); private ArtifactState state = ArtifactState.UNDEFINED; + private volatile boolean gone = false; public ArtifactState getState() { return state; @@ -22,4 +23,11 @@ public Queue getJobQueue() { return jobQueue; } + public boolean isGone() { + return gone; + } + + protected void makeGone() { + this.gone = true; + } } diff --git a/src/main/java/com/flowpowered/plugins/artifact/ArtifactManager.java b/src/main/java/com/flowpowered/plugins/artifact/ArtifactManager.java index bfaf54a..81632f3 100644 --- a/src/main/java/com/flowpowered/plugins/artifact/ArtifactManager.java +++ b/src/main/java/com/flowpowered/plugins/artifact/ArtifactManager.java @@ -25,8 +25,13 @@ public Future locate(String artifactName) { if (artifact == null) { enqueuePulse(artifactName); } else { - // FIXME: Race condition - we may enqueue the job too late. artifact.getJobQueue().add(job); + + if (artifact.isGone()) { + // Our job might be never processed, so let's try again. Better have 2 jobs than none. + // FIXME: Convert this to iteration, or we might get StackOverflows. + return locate(artifactName); + } } return job.getFuture(); } @@ -47,6 +52,7 @@ public void pulse(String artifactName) { if (job instanceof RemoveJob) { byName.remove(artifactName); + artifact.makeGone(); for (ArtifactJob j : artifact.getJobQueue()) { if (j instanceof LocateJob) { From 04adf745285f5a2ade4fba3f081fb53865fa4e56 Mon Sep 17 00:00:00 2001 From: Jack Huey Date: Mon, 11 Aug 2014 17:18:15 -0500 Subject: [PATCH 3/3] Don't use jobs. Instead be smart with ArtifactLoadState --- .../plugins/artifact/Artifact.java | 29 +---- .../plugins/artifact/ArtifactJob.java | 10 -- .../plugins/artifact/ArtifactLoadState.java | 27 +++++ .../plugins/artifact/ArtifactManager.java | 110 ++++++++++++------ .../plugins/artifact/ArtifactState.java | 5 - .../plugins/artifact/jobs/AbstractJob.java | 15 --- .../plugins/artifact/jobs/LocateJob.java | 14 --- .../plugins/artifact/jobs/RemoveJob.java | 13 --- 8 files changed, 105 insertions(+), 118 deletions(-) delete mode 100644 src/main/java/com/flowpowered/plugins/artifact/ArtifactJob.java create mode 100644 src/main/java/com/flowpowered/plugins/artifact/ArtifactLoadState.java delete mode 100644 src/main/java/com/flowpowered/plugins/artifact/ArtifactState.java delete mode 100644 src/main/java/com/flowpowered/plugins/artifact/jobs/AbstractJob.java delete mode 100644 src/main/java/com/flowpowered/plugins/artifact/jobs/LocateJob.java delete mode 100644 src/main/java/com/flowpowered/plugins/artifact/jobs/RemoveJob.java diff --git a/src/main/java/com/flowpowered/plugins/artifact/Artifact.java b/src/main/java/com/flowpowered/plugins/artifact/Artifact.java index 31b6ed9..448b33c 100644 --- a/src/main/java/com/flowpowered/plugins/artifact/Artifact.java +++ b/src/main/java/com/flowpowered/plugins/artifact/Artifact.java @@ -1,33 +1,12 @@ package com.flowpowered.plugins.artifact; -import java.util.Queue; -import java.util.concurrent.ConcurrentLinkedQueue; +import com.flowpowered.commons.SimpleFuture; public class Artifact { - private Queue jobQueue = new ConcurrentLinkedQueue<>(); - private ArtifactState state = ArtifactState.UNDEFINED; - private volatile boolean gone = false; + protected volatile SimpleFuture future; + protected volatile ArtifactLoadState state = ArtifactLoadState.UNDEFINED; - public ArtifactState getState() { + public ArtifactLoadState getState() { return state; } - - /** - * Should be only called from the thread that is currently executing {@link ArtifactManager#pulse(String)} - */ - public void setState(ArtifactState state) { - this.state = state; - } - - public Queue getJobQueue() { - return jobQueue; - } - - public boolean isGone() { - return gone; - } - - protected void makeGone() { - this.gone = true; - } } diff --git a/src/main/java/com/flowpowered/plugins/artifact/ArtifactJob.java b/src/main/java/com/flowpowered/plugins/artifact/ArtifactJob.java deleted file mode 100644 index 6deeebc..0000000 --- a/src/main/java/com/flowpowered/plugins/artifact/ArtifactJob.java +++ /dev/null @@ -1,10 +0,0 @@ -package com.flowpowered.plugins.artifact; - -import com.flowpowered.commons.SimpleFuture; - -public interface ArtifactJob { - - SimpleFuture getFuture(); - - void run(Artifact artifact); -} diff --git a/src/main/java/com/flowpowered/plugins/artifact/ArtifactLoadState.java b/src/main/java/com/flowpowered/plugins/artifact/ArtifactLoadState.java new file mode 100644 index 0000000..e1b50c3 --- /dev/null +++ b/src/main/java/com/flowpowered/plugins/artifact/ArtifactLoadState.java @@ -0,0 +1,27 @@ +package com.flowpowered.plugins.artifact; + +/** + * UNDEFINED -> LOCATED --> LOAD_REQUESTED -----> LOADING -> LOADED + * /| /\ /\ | /\ + * / | | | | + * / | | | | + * ---------------> | | \/ | + * REMOVED <- REMOVE_REQUESTED UNLOADED <- UNLOADING <- UNLOAD_REQUESTED + * <-------------- + */ +public enum ArtifactLoadState { + UNDEFINED, + + LOCATED, + + LOAD_REQUESTED, + LOADING, + LOADED, + + UNLOAD_REQUESTED, + UNLOADING, + UNLOADED, + + REMOVE_REQUESTED, + REMOVED; +} \ No newline at end of file diff --git a/src/main/java/com/flowpowered/plugins/artifact/ArtifactManager.java b/src/main/java/com/flowpowered/plugins/artifact/ArtifactManager.java index 81632f3..8f74338 100644 --- a/src/main/java/com/flowpowered/plugins/artifact/ArtifactManager.java +++ b/src/main/java/com/flowpowered/plugins/artifact/ArtifactManager.java @@ -4,36 +4,53 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.Future; -import com.flowpowered.plugins.artifact.jobs.LocateJob; -import com.flowpowered.plugins.artifact.jobs.RemoveJob; +import com.flowpowered.commons.SimpleFuture; public class ArtifactManager { - private ConcurrentMap byName = new ConcurrentHashMap<>(); - + private final ConcurrentMap byName = new ConcurrentHashMap<>(); /** * Makes the Manager try to find the artifact and start tracking it. * @return some Future, whose type will be specified once I figure out what I want it to be */ public Future locate(String artifactName) { - LocateJob job = new LocateJob(); - Artifact newArtifact = new Artifact(); - newArtifact.getJobQueue().add(job); - Artifact artifact = byName.putIfAbsent(artifactName, newArtifact); + SimpleFuture future = new SimpleFuture<>(); if (artifact == null) { + newArtifact.future = future; enqueuePulse(artifactName); - } else { - artifact.getJobQueue().add(job); + return future; + } + future.setResult(null); + return future; + } - if (artifact.isGone()) { - // Our job might be never processed, so let's try again. Better have 2 jobs than none. - // FIXME: Convert this to iteration, or we might get StackOverflows. - return locate(artifactName); - } + @SuppressWarnings("fallthrough") + public Future load(String artifactName) { + Artifact artifact = byName.get(artifactName); + if (artifact == null) { + return null; + } + switch (artifact.getState()) { + case UNDEFINED: + case REMOVED: + return null; + case LOCATED: + case UNLOAD_REQUESTED: + case REMOVE_REQUESTED: + case UNLOADING: + case UNLOADED: + artifact.state = ArtifactLoadState.LOAD_REQUESTED; + if (artifact.future != null) artifact.future.cancel(true); + artifact.future = new SimpleFuture<>(); + case LOAD_REQUESTED: + case LOADING: + case LOADED: + return artifact.future; + default: + throw new IllegalStateException("Unhandled state: " + artifact.getState()); } - return job.getFuture(); } /** @@ -42,28 +59,48 @@ public Future locate(String artifactName) { public void pulse(String artifactName) { Artifact artifact = byName.get(artifactName); if (artifact == null) { - // Should not happen, only we're allowed to set it to null and wouldn't pulse after that - throw new IllegalStateException("pulsed on nonexistent artifact"); + return; } - ArtifactJob job = artifact.getJobQueue().poll(); - if (job != null) { - // TODO: Job merging - job.run(artifact); - - if (job instanceof RemoveJob) { + switch (artifact.getState()) { + case UNDEFINED: + // TODO: locate + artifact.future.setResult(null); + artifact.future = null; + artifact.state = ArtifactLoadState.LOCATED; + break; + case LOAD_REQUESTED: + SimpleFuture loadFuture = artifact.future; + artifact.state = ArtifactLoadState.LOADING; + // TODO: load + loadFuture.setResult(null); + artifact.state = ArtifactLoadState.LOADED; + break; + case UNLOAD_REQUESTED: + SimpleFuture unloadFuture = artifact.future; + artifact.state = ArtifactLoadState.UNLOADING; + // TODO: unloaded + unloadFuture.setResult(null); + artifact.state = ArtifactLoadState.UNLOADED; + break; + case REMOVE_REQUESTED: + SimpleFuture removeFuture = artifact.future; + artifact.state = ArtifactLoadState.REMOVED; byName.remove(artifactName); - artifact.makeGone(); + removeFuture.setResult(null); + break; - for (ArtifactJob j : artifact.getJobQueue()) { - if (j instanceof LocateJob) { - locate(artifactName); - // TODO: add the rest of the queue to the new artifact? - break; - } - } - - return; // Don't requeue ourselves; - } + case LOCATED: + case LOADED: + case UNLOADED: + // The Artifact is "stable" and shouldn't change + break; + case LOADING: + case UNLOADING: + throw new IllegalStateException("Pulsing the same artifact multiple times at once!"); + case REMOVED: + throw new IllegalStateException("Removed artifact being pulsed!"); + default: + throw new IllegalStateException("Unhandled state: " + artifact.getState()); } enqueuePulse(artifactName); } @@ -73,6 +110,7 @@ public void pulse(String artifactName) { * The definition of "soon" is implementation-specific. */ protected void enqueuePulse(String artifactName) { - + // Allows an artifact to only be queued once + // aLinkedHashSet.add(artifactName); } } diff --git a/src/main/java/com/flowpowered/plugins/artifact/ArtifactState.java b/src/main/java/com/flowpowered/plugins/artifact/ArtifactState.java deleted file mode 100644 index cd123a4..0000000 --- a/src/main/java/com/flowpowered/plugins/artifact/ArtifactState.java +++ /dev/null @@ -1,5 +0,0 @@ -package com.flowpowered.plugins.artifact; - -public enum ArtifactState { - UNDEFINED, LOCATED, LOADING, LOADED, WAITING_FOR_DEPS, MISSING_DEPS, RESOLVED, UNLOADING, UNLOADED -} diff --git a/src/main/java/com/flowpowered/plugins/artifact/jobs/AbstractJob.java b/src/main/java/com/flowpowered/plugins/artifact/jobs/AbstractJob.java deleted file mode 100644 index 4d03988..0000000 --- a/src/main/java/com/flowpowered/plugins/artifact/jobs/AbstractJob.java +++ /dev/null @@ -1,15 +0,0 @@ -package com.flowpowered.plugins.artifact.jobs; - -import com.flowpowered.commons.SimpleFuture; - -import com.flowpowered.plugins.artifact.ArtifactJob; - -public abstract class AbstractJob implements ArtifactJob { - protected SimpleFuture future = new SimpleFuture<>(); - - @Override - public SimpleFuture getFuture() { - return future; - } - -} \ No newline at end of file diff --git a/src/main/java/com/flowpowered/plugins/artifact/jobs/LocateJob.java b/src/main/java/com/flowpowered/plugins/artifact/jobs/LocateJob.java deleted file mode 100644 index a0e80ad..0000000 --- a/src/main/java/com/flowpowered/plugins/artifact/jobs/LocateJob.java +++ /dev/null @@ -1,14 +0,0 @@ -package com.flowpowered.plugins.artifact.jobs; - -import com.flowpowered.plugins.artifact.Artifact; -import com.flowpowered.plugins.artifact.ArtifactState; - -public class LocateJob extends AbstractJob { - @Override - public void run(Artifact artifact) { - // TODO Auto-generated method stub - artifact.setState(ArtifactState.LOCATED); - future.setResult(null); - } - -} diff --git a/src/main/java/com/flowpowered/plugins/artifact/jobs/RemoveJob.java b/src/main/java/com/flowpowered/plugins/artifact/jobs/RemoveJob.java deleted file mode 100644 index 9032474..0000000 --- a/src/main/java/com/flowpowered/plugins/artifact/jobs/RemoveJob.java +++ /dev/null @@ -1,13 +0,0 @@ -package com.flowpowered.plugins.artifact.jobs; - -import com.flowpowered.plugins.artifact.Artifact; - -public class RemoveJob extends AbstractJob { - - @Override - public void run(Artifact artifact) { - // TODO Auto-generated method stub - future.setResult(null); - } - -}