Skip to content

Commit

Permalink
Report only unique warnings (#6372)
Browse files Browse the repository at this point in the history
This change makes sure that reported warnings are unique, based on the value of internal clock tick and ignoring differences in reassignments.

Before:
![Screenshot from 2023-04-20 15-42-55](https://user-images.githubusercontent.com/292128/233415710-925c1045-37c7-49f5-9bc3-bfbfd30270a3.png)
After:
![Screenshot from 2023-04-20 15-27-27](https://user-images.githubusercontent.com/292128/233415807-8cb67bc2-ac37-4db7-924e-ae7619074b5b.png)

On the positive side, no further changes, like in LS, have to be done.


Closes #6257.
  • Loading branch information
hubertp authored Apr 28, 2023
1 parent c0679af commit c6790f1
Show file tree
Hide file tree
Showing 28 changed files with 406 additions and 279 deletions.
8 changes: 5 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@
- [Optimize Atom storage layouts][3862]
- [Make instance methods callable like statics for builtin types][4077]
- [Convert large longs to doubles, safely, for host calls][4099]
- [Consistent ordering with comparators](4067)
- [Consistent ordering with comparators][4067]
- [Profile engine startup][4110]
- [Report type of polyglot values][4111]
- [Engine can now recover from serialization failures][5591]
Expand All @@ -709,6 +709,7 @@
- [Vector.sort handles incomparable types][5998]
- [Removing need for asynchronous thread to execute ResourceManager
finalizers][6335]
- [Warning.get_all returns only unique warnings][6372]

[3227]: https://github.com/enso-org/enso/pull/3227
[3248]: https://github.com/enso-org/enso/pull/3248
Expand Down Expand Up @@ -797,9 +798,9 @@
[4048]: https://github.com/enso-org/enso/pull/4048
[4049]: https://github.com/enso-org/enso/pull/4049
[4056]: https://github.com/enso-org/enso/pull/4056
[4067]: https://github.com/enso-org/enso/pull/4067
[4077]: https://github.com/enso-org/enso/pull/4077
[4099]: https://github.com/enso-org/enso/pull/4099
[4067]: https://github.com/enso-org/enso/pull/4067
[4110]: https://github.com/enso-org/enso/pull/4110
[4111]: https://github.com/enso-org/enso/pull/4111
[5591]: https://github.com/enso-org/enso/pull/5591
Expand All @@ -811,11 +812,12 @@
[5791]: https://github.com/enso-org/enso/pull/5791
[5900]: https://github.com/enso-org/enso/pull/5900
[5966]: https://github.com/enso-org/enso/pull/5966
[5998]: https://github.com/enso-org/enso/pull/5998
[6067]: https://github.com/enso-org/enso/pull/6067
[6151]: https://github.com/enso-org/enso/pull/6151
[6171]: https://github.com/enso-org/enso/pull/6171
[5998]: https://github.com/enso-org/enso/pull/5998
[6335]: https://github.com/enso-org/enso/pull/6335
[6372]: https://github.com/enso-org/enso/pull/6372

# Enso 2.0.0-alpha.18 (2021-10-12)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package org.enso.interpreter.bench.benchmarks.semantic;

import org.enso.interpreter.test.TestBase;
import org.enso.polyglot.MethodNames;
import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.Value;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.TearDown;
import org.openjdk.jmh.annotations.Warmup;
import org.openjdk.jmh.infra.BenchmarkParams;

import java.io.IOException;
import java.util.Objects;
import java.util.concurrent.TimeUnit;

@BenchmarkMode(Mode.AverageTime)
@Fork(1)
@Warmup(iterations = 5, time = 1)
@Measurement(iterations = 3, time = 3)
@OutputTimeUnit(TimeUnit.MILLISECONDS)
@State(Scope.Benchmark)
public class WarningBenchmarks extends TestBase {
private static final int INPUT_VEC_SIZE = 10000;
private Context ctx;
private Value vecSumBench;

private Value createVec;
private Value noWarningsVec;
private Value sameWarningVec;

private Value elem;

private Value elemWithWarning;

private String benchmarkName;

@Setup
public void initializeBench(BenchmarkParams params) throws IOException {
ctx = createDefaultContext();

benchmarkName = params.getBenchmark().replaceFirst(".*\\.", "");

var code = """
from Standard.Base import all
vec_sum_bench : Vector Integer -> Integer
vec_sum_bench vec =
vec.fold 0 (x->y->x+y)
create_vec size elem =
Vector.fill size elem
elem =
42
elem_with_warning =
x = 42
Warning.attach "Foo!" x
""";
var src = SrcUtil.source(benchmarkName, code);
Value module = ctx.eval(src);
vecSumBench = Objects.requireNonNull(module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "vec_sum_bench"));
createVec = Objects.requireNonNull(module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "create_vec"));
elem = Objects.requireNonNull(module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "elem"));
elemWithWarning = Objects.requireNonNull(module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "elem_with_warning"));
noWarningsVec = createVec.execute(INPUT_VEC_SIZE, elem);
sameWarningVec = createVec.execute(INPUT_VEC_SIZE, elemWithWarning);
}

@TearDown
public void cleanup() {
ctx.close(true);
}

@Benchmark
public void noWarningsVecSum() {
Value res = vecSumBench.execute(noWarningsVec);
checkResult(res);
}

@Benchmark
public void sameWarningVecSum() {
Value res = vecSumBench.execute(sameWarningVec);
checkResult(res);
}

private static void checkResult(Value res) {
if (res.asInt() != INPUT_VEC_SIZE*42) {
throw new AssertionError("Expected result: " + INPUT_VEC_SIZE*42 + ", got: " + res.asInt());
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ Object doWarning(
int thatArgumentPosition,
@Cached IndirectInvokeConversionNode childDispatch) {
arguments[thatArgumentPosition] = that.getValue();
ArrayRope<Warning> warnings = that.getReassignedWarnings(this);
ArrayRope<Warning> warnings = that.getReassignedWarningsAsRope(this);
Object result =
childDispatch.execute(
frame,
Expand All @@ -156,7 +156,7 @@ Object doWarning(
argumentsExecutionMode,
isTail,
thatArgumentPosition);
return WithWarnings.prependTo(result, warnings);
return WithWarnings.appendTo(result, warnings);
}

@Specialization(guards = "interop.isString(that)")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ Object doWarning(
int thisArgumentPosition,
@Cached IndirectInvokeMethodNode childDispatch) {
arguments[thisArgumentPosition] = self.getValue();
ArrayRope<Warning> warnings = self.getReassignedWarnings(this);
ArrayRope<Warning> warnings = self.getReassignedWarningsAsRope(this);
Object result =
childDispatch.execute(
frame,
Expand All @@ -134,7 +134,7 @@ Object doWarning(
argumentsExecutionMode,
isTail,
thisArgumentPosition);
return WithWarnings.prependTo(result, warnings);
return WithWarnings.appendTo(result, warnings);
}

@Specialization
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ public Object invokeWarnings(
if (result instanceof DataflowError) {
return result;
} else if (result instanceof WithWarnings withWarnings) {
return withWarnings.prepend(extracted);
return withWarnings.append(extracted);
} else {
return WithWarnings.wrap(result, extracted);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,10 @@ Object doWarning(
}
}
arguments[thatArgumentPosition] = that.getValue();
ArrayRope<Warning> warnings = that.getReassignedWarnings(this);
ArrayRope<Warning> warnings = that.getReassignedWarningsAsRope(this);
Object result =
childDispatch.execute(frame, state, conversion, self, that.getValue(), arguments);
return WithWarnings.prependTo(result, warnings);
return WithWarnings.appendTo(result, warnings);
}

@Specialization(guards = "interop.isString(that)")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ Object doWarning(
arguments[thisArgumentPosition] = selfWithoutWarnings;

Object result = childDispatch.execute(frame, state, symbol, selfWithoutWarnings, arguments);
return WithWarnings.prependTo(result, arrOfWarnings);
return WithWarnings.appendTo(result, arrOfWarnings);
}

@ExplodeLoop
Expand Down Expand Up @@ -327,7 +327,7 @@ Object doPolyglot(
Object res = hostMethodCallNode.execute(polyglotCallType, symbol.getName(), self, args);
if (anyWarnings) {
anyWarningsProfile.enter();
res = WithWarnings.prependTo(res, accumulatedWarnings);
res = WithWarnings.appendTo(res, accumulatedWarnings);
}
return res;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ public TruffleLogger getLogger(Class<?> klass) {
*
* <p>The counter is used to track the creation time of warnings.
*/
public long clockTick() {
public long nextSequenceId() {
return clock.getAndIncrement();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import java.util.Arrays;
import org.enso.interpreter.runtime.error.WithWarnings;
import org.graalvm.collections.EconomicSet;

/** A primitive boxed array type for use in the runtime. */
@ExportLibrary(InteropLibrary.class)
Expand All @@ -28,6 +29,7 @@
public final class Array implements TruffleObject {
private final Object[] items;
private Boolean withWarnings;
private Warning[] cachedWarnings;

/**
* Creates a new array
Expand Down Expand Up @@ -203,13 +205,22 @@ boolean hasWarnings(@CachedLibrary(limit = "3") WarningsLibrary warnings) {
@ExportMessage
Warning[] getWarnings(Node location, @CachedLibrary(limit = "3") WarningsLibrary warnings)
throws UnsupportedMessageException {
ArrayRope<Warning> ropeOfWarnings = new ArrayRope<>();
if (cachedWarnings == null) {
cachedWarnings = Warning.fromSetToArray(collectAllWarnings(warnings, location));
}
return cachedWarnings;
}

@CompilerDirectives.TruffleBoundary
private EconomicSet<Warning> collectAllWarnings(WarningsLibrary warnings, Node location)
throws UnsupportedMessageException {
EconomicSet<Warning> setOfWarnings = EconomicSet.create(new WithWarnings.WarningEquivalence());
for (int i = 0; i < items.length; i++) {
if (warnings.hasWarnings(items[i])) {
ropeOfWarnings = ropeOfWarnings.prepend(warnings.getWarnings(items[i], location));
setOfWarnings.addAll(Arrays.asList(warnings.getWarnings(items[i], location)));
}
}
return ropeOfWarnings.toArray(Warning[]::new);
return setOfWarnings;
}

@ExportMessage
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.enso.interpreter.runtime.data;

import com.oracle.truffle.api.CompilerDirectives;

import java.util.Arrays;
import java.util.function.Function;

Expand Down Expand Up @@ -33,6 +35,7 @@ public final ArrayRope<T> prepend(T... items) {
return new ArrayRope<>(new ConcatSegment<>(new ArraySegment<>(items), this.segment));
}

@CompilerDirectives.TruffleBoundary
public T[] toArray(Function<Integer, T[]> genArray) {
T[] res = genArray.apply(size());
writeArray(res);
Expand Down
Loading

0 comments on commit c6790f1

Please sign in to comment.