Skip to content

Commit 2581611

Browse files
ctruedenctrueden
authored
Merge pull request #43 from scijava/scijava/scijava-ops-engine/progress-reporting
Progress Reporting Implementation
2 parents 05a1e18 + 11f50d2 commit 2581611

25 files changed

Lines changed: 1701 additions & 49 deletions

File tree

‎pom.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
<module>scijava/scijava-ops-serviceloader</module>
5555
<module>scijava/scijava-ops-spi</module>
5656
<module>scijava/scijava-persist</module>
57+
<module>scijava/scijava-progress</module>
5758
<module>scijava/scijava-struct</module>
5859
<module>scijava/scijava-taglets</module>
5960
<module>scijava/scijava-testutil</module>

‎scijava/scijava-ops-api/src/main/java/org/scijava/ops/api/OpExecution.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
package org.scijava.ops.api;
2+
3+
import java.util.concurrent.CompletableFuture;
4+
import java.util.concurrent.Future;
5+
6+
public class OpExecution {
7+
8+
private final RichOp<?> op;
9+
private final CompletableFuture<Object> outContainer;
10+
11+
public OpExecution(RichOp<?> op) {
12+
this.op = op;
13+
this.outContainer = new CompletableFuture<>();
14+
}
15+
16+
public RichOp<?> op() {
17+
return op;
18+
}
19+
20+
public void recordCompletion(Object output) {
21+
outContainer.complete(output);
22+
}
23+
24+
public Future<Object> output() {
25+
return outContainer;
26+
}
27+
28+
29+
}

‎scijava/scijava-ops-api/src/main/java/org/scijava/ops/api/OpHistory.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
package org.scijava.ops.api;
33

44
import java.util.List;
5+
import java.util.Set;
56

67
/**
78
* Log describing each execution of an Op. This class is designed to answer two
@@ -51,19 +52,18 @@ default String signatureOf(Object op) {
5152

5253
// -- HISTORY MAINTENANCE API -- //
5354

54-
/**
55-
* Logs a {@link RichOp} execution in the history
56-
*
57-
* @param op the Op executed to produce {@code output}
58-
* @param output the output produced by {@code op}
59-
*/
60-
void addExecution(RichOp<?> op, Object output);
61-
6255
/**
6356
* Logs the creation of {@link RichOp}
6457
*
6558
* @param op the {@link RichOp} containing relevant information
6659
*/
6760
void logOp(RichOp<?> op);
6861

62+
/**
63+
* Logs the {@link Object} output of the {@link RichOp} {@code op}.
64+
* @param op the {@link RichOp} producing {@code output}
65+
* @param output the {@link Object} output of {@code e}
66+
*/
67+
void logOutput(RichOp<?> op, Object output);
68+
6969
}

‎scijava/scijava-ops-engine/pom.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,11 @@
141141
<artifactId>scijava-ops-serviceloader</artifactId>
142142
<version>${project.version}</version>
143143
</dependency>
144+
<dependency>
145+
<groupId>org.scijava</groupId>
146+
<artifactId>scijava-progress</artifactId>
147+
<version>${project.version}</version>
148+
</dependency>
144149
<dependency>
145150
<groupId>org.scijava</groupId>
146151
<artifactId>scijava-struct</artifactId>

‎scijava/scijava-ops-engine/src/main/java/module-info.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,20 @@
3030
opens org.scijava.ops.engine.util to therapi.runtime.javadoc;
3131
opens org.scijava.ops.engine.math to therapi.runtime.javadoc;
3232

33+
requires java.compiler;
34+
3335
requires org.scijava;
3436
requires org.scijava.discovery;
3537
requires org.scijava.function;
38+
requires org.scijava.progress;
3639
requires org.scijava.struct;
3740
requires transitive org.scijava.ops.api;
3841
requires org.scijava.ops.serviceloader;
3942
requires org.scijava.ops.spi;
4043
requires org.scijava.types;
44+
4145
requires javassist;
42-
requires java.compiler;
46+
4347
requires therapi.runtime.javadoc;
4448

4549
uses javax.annotation.processing.Processor;

‎scijava/scijava-ops-engine/src/main/java/org/scijava/ops/engine/impl/DefaultOpEnvironment.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -477,10 +477,9 @@ private OpInstance<?> instantiateOp(final OpCandidate candidate,
477477
Hints hints)
478478
{
479479
final List<RichOp<?>> conditions = resolveOpDependencies(candidate, hints);
480-
final List<InfoChain> depChains = conditions.stream().map(op -> op
481-
.metadata().info()).collect(Collectors.toList());
482-
final InfoChain chain = new InfoChain(candidate.opInfo(), depChains);
483-
return chain.op();
480+
InfoChain adaptorChain = new DependencyInstantiatedInfoChain(candidate
481+
.opInfo(), conditions);
482+
return adaptorChain.op();
484483
}
485484

486485
/**

‎scijava/scijava-ops-engine/src/main/java/org/scijava/ops/engine/impl/DefaultOpHistory.java‎

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,15 @@
22
package org.scijava.ops.engine.impl;
33

44
import java.util.ArrayList;
5+
import java.util.Collections;
6+
import java.util.HashSet;
57
import java.util.List;
68
import java.util.Map;
9+
import java.util.Set;
710
import java.util.WeakHashMap;
811

912
import org.scijava.ops.api.InfoChain;
13+
import org.scijava.ops.api.OpExecution;
1014
import org.scijava.ops.api.OpHistory;
1115
import org.scijava.ops.api.OpInfo;
1216
import org.scijava.ops.api.RichOp;
@@ -39,8 +43,9 @@ public class DefaultOpHistory extends AbstractService implements OpHistory {
3943
// -- DATA STRCUTURES -- //
4044

4145
/**
42-
* {@link Map} responsible for recording the {@link InfoChain} of {@link OpInfo}s
43-
* involved to produce the result of a particular matching call
46+
* {@link Map} responsible for recording the {@link InfoChain} of
47+
* {@link OpInfo}s involved to produce the result of a particular matching
48+
* call
4449
*/
4550
private final Map<RichOp<?>, InfoChain> dependencyChain = new WeakHashMap<>();
4651

@@ -59,7 +64,7 @@ public class DefaultOpHistory extends AbstractService implements OpHistory {
5964
public List<RichOp<?>> executionsUpon(Object o) {
6065
if (o.getClass().isPrimitive()) throw new IllegalArgumentException(
6166
"Cannot determine the executions upon a primitive as they are passed by reference!");
62-
return mutationMap.get(o);
67+
return mutationMap.getOrDefault(o, Collections.emptyList());
6368
}
6469

6570
@Override
@@ -69,28 +74,15 @@ public InfoChain opExecutionChain(Object op) {
6974

7075
// -- HISTORY MAINTENANCE API -- //
7176

72-
/**
73-
* Logs an Op execution in the history
74-
* <p>
75-
* TODO: It would be nice if different Objects returned different Objects with
76-
* the same hash code would hash differently. For example, if two Ops return a
77-
* {@link Double} of the same value, they will appear as the same Object, and
78-
* asking for the execution history on either of the {@link Object}s will
79-
* suggest that both executions mutated both {@link Object}s. This would
80-
* really hamper the simplicity of the implementation, however.
81-
*
82-
* @param op the {@link RichOp} being executed
83-
* @param output the output of the Op execution
84-
*/
8577
@Override
86-
public void addExecution(RichOp<?> op, Object output) {
87-
if (!mutationMap.containsKey(output)) updateList(output);
88-
resolveExecution(op, output);
78+
public void logOp(RichOp<?> op) {
79+
dependencyChain.put(op, op.infoChain());
8980
}
9081

9182
@Override
92-
public void logOp(RichOp<?> op) {
93-
dependencyChain.put(op, op.infoChain());
83+
public void logOutput(RichOp<?> op, Object output) {
84+
if (!mutationMap.containsKey(output)) updateList(output);
85+
resolveExecution(op, output);
9486
}
9587

9688
// -- HELPER METHODS -- //

‎scijava/scijava-ops-engine/src/main/java/org/scijava/ops/engine/impl/DependencyInstantiatedInfoChain.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,7 @@ public DependencyInstantiatedInfoChain(OpInfo info, List<RichOp<?>> dependencies
2525

2626
@Override
2727
protected Object generateOp() {
28-
List<Object> dependencyInstances = dependencies.stream() //
29-
.map(d -> d.op()) //
30-
.collect(Collectors.toList());
31-
return info().createOpInstance(dependencyInstances).object();
28+
return info().createOpInstance(dependencies).object();
3229
}
3330

3431
public static List<InfoChain> chainsFrom(List<RichOp<?>> dependencies) {

‎scijava/scijava-ops-engine/src/main/java/org/scijava/ops/engine/matcher/impl/AbstractRichOp.java‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import org.scijava.ops.api.OpMetadata;
88
import org.scijava.ops.api.RichOp;
99
import org.scijava.ops.api.features.BaseOpHints.History;
10+
import org.scijava.progress.Progress;
1011

1112
/**
1213
* An abstract implementation of {@link RichOp}. While this class has <b>no
@@ -43,14 +44,15 @@ public OpMetadata metadata() {
4344
}
4445

4546
@Override
46-
public void preprocess(Object... inputs) {}
47+
public void preprocess(Object... inputs) {
48+
Progress.register(this);
49+
}
4750

4851
@Override
4952
public void postprocess(Object output) {
5053
// Log a new execution
51-
if (!metadata.hints().contains(History.SKIP_RECORDING)) {
52-
metadata.history().addExecution(this, output);
53-
}
54+
metadata.history().logOutput(this, output);
55+
Progress.complete();
5456
}
5557

5658
}

‎scijava/scijava-ops-engine/src/test/java/org/scijava/ops/engine/impl/ProvenanceTest.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,9 @@ public void testMappingProvenance() {
134134
Thing out = ops.op("test.provenanceMapper").input(array).outType(
135135
Thing.class).apply();
136136

137-
// Assert only one execution upon this Object
137+
// Assert two executions upon this Object, once from the mapped function, once from the mapper
138138
List<RichOp<?>> executionsUpon = ops.history().executionsUpon(out);
139-
Assert.assertEquals(1, executionsUpon.size());
139+
Assert.assertEquals(2, executionsUpon.size());
140140
}
141141

142142
@Test
@@ -176,15 +176,15 @@ public void testMappingProvenanceAndCaching() {
176176
Thing out = ops.op("test.provenanceMapper").input(array).outType(
177177
Thing.class).apply(hints);
178178

179-
// Assert only one run of the Base Op
179+
// Assert that two Ops operated on the return.
180180
List<RichOp<?>> history = ops.history().executionsUpon(out);
181-
Assert.assertEquals(1, history.size());
181+
Assert.assertEquals(2, history.size());
182182

183-
// Run the mapped Op, assert still one run on the mapper
183+
// Run the mapped Op, assert still two runs on the mapper
184184
Thing out1 = ops.op("test.provenanceMapped").input(2.).outType(Thing.class)
185185
.apply(hints);
186186
history = ops.history().executionsUpon(out);
187-
Assert.assertEquals(1, history.size());
187+
Assert.assertEquals(2, history.size());
188188
// Assert one run on the mapped Op as well
189189
history = ops.history().executionsUpon(out1);
190190
Assert.assertEquals(1, history.size());

0 commit comments

Comments
 (0)