Make MethodProbes use MethodHandles over reflection (HZ-3024)
#25279
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JackPGreen
added
Type: Enhancement
Team: Core
Source: Internal
|
The job Click to expand the log file---------ERRORS----------- -------------------------- [ERROR] Tests run: 18, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.115 s <<< FAILURE! -- in com.hazelcast.internal.metrics.impl.RegisterAnnotatedMethodsTest -------------------------- [ERROR] com.hazelcast.internal.metrics.impl.RegisterAnnotatedMethodsTest.register_staticMethod -- Time elapsed: 0.043 s <<< FAILURE! -------------------------- [ERROR] Failures: -------------------------- [ERROR] RegisterAnnotatedMethodsTest.register_staticMethod:312 expected:<10> but was:<0> -------------------------- [ERROR] Tests run: 4663, Failures: 1, Errors: 0, Skipped: 11 -------------------------- [ERROR] There are test failures. -------------------------- -------------------------- -------TEST FAILURE------- -------------------------- [INFO] Results: [INFO] [ERROR] Failures: [ERROR] RegisterAnnotatedMethodsTest.register_staticMethod:312 expected:<10> but was:<0> [INFO] [ERROR] Tests run: 4663, Failures: 1, Errors: 0, Skipped: 11 [INFO] [WARNING] Corrupted channel by directly writing to native stream in forked JVM 8. See FAQ web page and the dump file /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-builder/hazelcast/target/surefire-reports/2023-08-23T23-31-16_736-jvmRun8.dumpstream |
|
The job Click to expand the log file---------ERRORS----------- -------------------------- [ERROR] Failed to execute goal pl.project13.maven:git-commit-id-plugin:4.9.10:revision (default) on project hazelcast: Could not complete Mojo execution... Missing tree 2a16e70f898cd00b6ae18a9967010c239ffe6d55 -> [Help 1] -------------------------- [ERROR] -------------------------- [ERROR] Re-run Maven using the -X switch to enable full debug logging. -------------------------- [ERROR] -------------------------- [ERROR] For more information about the errors and possible solutions, please read the following articles: -------------------------- [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException -------------------------- [ERROR] -------------------------- [ERROR] After correcting the problems, you can resume the build with the command -------------------------- [ERROR] mvn -rf :hazelcast -------------------------- |
|
run-lab-run |
|
The job Click to expand the log file---------ERRORS-----------
--------------------------
[ERROR] Tests run: 2, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 13.39 s <<< FAILURE! -- in com.hazelcast.client.cache.impl.nearcache.ClientCacheNearCacheCacheOnUpdateTest
--------------------------
[ERROR] com.hazelcast.client.cache.impl.nearcache.ClientCacheNearCacheCacheOnUpdateTest.with_cacheOnUpdate_policy_concurrently_updated_near_cache_does_not_cause_any_miss -- Time elapsed: 13.34 s <<< FAILURE!
--------------------------
[ERROR] Failures:
--------------------------
[ERROR] ClientCacheNearCacheCacheOnUpdateTest.with_cacheOnUpdate_policy_concurrently_updated_near_cache_does_not_cause_any_miss:68 100, NearCacheStatsImpl{ownedEntryCount=100, ownedEntryMemoryCost=11300, creationTime=1693342079782, hits=51111595, misses=332, ratio=15395058.7%, evictions=0, expirations=0, invalidations=101, invalidationRequests=103, lastPersistenceTime=0, persistenceCount=0, lastPersistenceDuration=0, lastPersistenceWrittenBytes=0, lastPersistenceKeyCount=0, lastPersistenceFailure=''}, NearCacheConfig{name=ClientCache, inMemoryFormat=BINARY, invalidateOnChange=true, timeToLiveSeconds=0, maxIdleSeconds=0, evictionConfig=EvictionConfig{size=2147483647, maxSizePolicy=ENTRY_COUNT, evictionPolicy=LRU, comparatorClassName=null, comparator=null}, cacheLocalEntries=false, localUpdatePolicy=CACHE_ON_UPDATE, preloaderConfig=NearCachePreloaderConfig{enabled=false, directory=, storeInitialDelaySeconds=600, storeIntervalSeconds=600}} expected:<0> but was:<332>
--------------------------
[ERROR] Tests run: 58549, Failures: 1, Errors: 0, Skipped: 243
--------------------------
[ERROR] There are test failures.
--------------------------
[ERROR] Tests run: 17, Failures: 1, Errors: 0, Skipped: 1, Time elapsed: 191.0 s <<< FAILURE! -- in com.hazelcast.jet.kinesis.KinesisIntegrationTest
--------------------------
[ERROR] com.hazelcast.jet.kinesis.KinesisIntegrationTest.dynamicStream_1Shard_splitsDuringData -- Time elapsed: 127.0 s <<< FAILURE!
--------------------------
[ERROR] Failures:
--------------------------
[ERROR] KinesisIntegrationTest.dynamicStream_1Shard_splitsDuringData:324->dynamicStream_splitsDuringData:354->assertMessages:637->AbstractKinesisTest.assertMessages:163->HazelcastTestSupport.assertTrueEventually:1269->HazelcastTestSupport.assertTrueEventually:1165->AbstractKinesisTest.lambda$assertMessages$3:180 Messages for key 241 differ!
--------------------------
[ERROR] Tests run: 25, Failures: 1, Errors: 0, Skipped: 3
--------------------------
[ERROR] There are test failures.
--------------------------
--------------------------
-------TEST FAILURE-------
--------------------------
[INFO] Results:
[INFO]
[ERROR] Failures:
[ERROR] ClientCacheNearCacheCacheOnUpdateTest.with_cacheOnUpdate_policy_concurrently_updated_near_cache_does_not_cause_any_miss:68 100, NearCacheStatsImpl{ownedEntryCount=100, ownedEntryMemoryCost=11300, creationTime=1693342079782, hits=51111595, misses=332, ratio=15395058.7%, evictions=0, expirations=0, invalidations=101, invalidationRequests=103, lastPersistenceTime=0, persistenceCount=0, lastPersistenceDuration=0, lastPersistenceWrittenBytes=0, lastPersistenceKeyCount=0, lastPersistenceFailure=''}, NearCacheConfig{name=ClientCache, inMemoryFormat=BINARY, invalidateOnChange=true, timeToLiveSeconds=0, maxIdleSeconds=0, evictionConfig=EvictionConfig{size=2147483647, maxSizePolicy=ENTRY_COUNT, evictionPolicy=LRU, comparatorClassName=null, comparator=null}, cacheLocalEntries=false, localUpdatePolicy=CACHE_ON_UPDATE, preloaderConfig=NearCachePreloaderConfig{enabled=false, directory=, storeInitialDelaySeconds=600, storeIntervalSeconds=600}} expected:<0> but was:<332>
[INFO]
[ERROR] Tests run: 58549, Failures: 1, Errors: 0, Skipped: 243
[INFO]
|
|
run-lab-run |
|
Following discussion with @pveentjer, the primary aim is not to improve execution time, but reduce gc overhead. Our benchmarks on the relative gc "litter" between native / reflective / method handle invocations do not line up - his show real improvement, mine don't - so more investigation required from me on this. |
pveentjer
reviewed
Sep 25, 2023
| switch (type) { | ||
| case TYPE_PRIMITIVE_LONG: | ||
| return ((Number) method.invoke(source, EMPTY_ARGS)).longValue(); | ||
| case TYPE_LONG_PRIMITIVE: |
There was a problem hiding this comment.
The optimization should be applied to all fields of type byte, short, int, long. And that is why the type system needs to be enriched so that it includes all these primitives.
Otherwise this will only work for a primitive long field; but not for any other of the other primitive types.
perhaps the confusion is caused by the fact that this is the LongMethodProbe; but this probe can be used for fields of type byte, short, int and long.
There was a problem hiding this comment.
The optimization should be applied to all fields of type byte, short, int, long. And that is why the type system needs to be enriched so that it includes all these primitives.
I'm not sure that this is required, but I don't think it's in scope for this issue - do you want to raise it separately?
perhaps the confusion is caused by the fact that this is the LongMethodProbe; but this probe can be used for fields of type byte, short, int and long.
I hadn't considered this - I've added com.hazelcast.internal.metrics.impl.ProbeUtilsTest.main(String[]) to print all of the types a Probe has been found against.
There was a problem hiding this comment.
It in in scope of this issue. You want to remove the litter.
There was a problem hiding this comment.
What is needed here is something like this:
@Override
public long get(S source) throws Exception {
switch (type) {
case TYPE_PRIMITIVE_LONG:
return (long) METHOD_HANDLE.invokeExact(source);
case TYPE_PRIMITIVE_INT:
return (int) METHOD_HANDLE.invokeExact(source);
case TYPE_PRIMITIVE_BYTE:
return (byte) METHOD_HANDLE.invokeExact(source);
case TYPE_PRIMITIVE_CHAR:
return (char) METHOD_HANDLE.invokeExact(source);
case TYPE_LONG_NUMBER:
Number longNumber = (Number) method.invoke(source, EMPTY_ARGS);
return longNumber == null ? 0 : longNumber.longValue();
case TYPE_MAP:
Map<?, ?> map = (Map<?, ?>) method.invoke(source, EMPTY_ARGS);
return map == null ? 0 : map.size();
case TYPE_COLLECTION:
Collection<?> collection = (Collection<?>) method.invoke(source, EMPTY_ARGS);
return collection == null ? 0 : collection.size();
case TYPE_COUNTER:
Counter counter = (Counter) method.invoke(source, EMPTY_ARGS);
return counter == null ? 0 : counter.get();
case TYPE_SEMAPHORE:
Semaphore semaphore = (Semaphore) method.invoke(source, EMPTY_ARGS);
return semaphore == null ? 0 : semaphore.availablePermits();
default:
throw new IllegalStateException("Unrecognized type:" + type);
}```
There was a problem hiding this comment.
It in in scope of this issue. You want to remove the litter.
Do you have a benchmark showing this is required? From the MethodHandleMismatchBenchmark you sent me, I don't think this is required when using java.lang.invoke.MethodType.changeReturnType(Class<?>).
I get no litter with this.
pveentjer
reviewed
Sep 25, 2023
| super(method, probe, type, sourceMetadata); | ||
| } | ||
|
|
||
| @Override | ||
| public double get(S source) throws Exception { | ||
| public double get(final S source) throws Throwable { | ||
| switch (type) { |
There was a problem hiding this comment.
Here is the same problem as with the long. The float type is missing. So there needs to be support for a primitive float so that boxing is prevented.
There was a problem hiding this comment.
Here is the same problem as with the long. The float type is missing. So there needs to be support for a primitive float so that boxing is prevented.
I don't think this is the case? A float will be cast to a double, not boxed, and that doesn't have any (significant) performance impact.
Using the same benchmarks as before, a LongProbe accessing methods returning long, int, Long & Integer:
Benchmark (field) Mode Cnt Score Error Units
MethodProbeBenchmark.benchmark longMethod avgt 1000 5.636 ± 0.008 ns/op
MethodProbeBenchmark.benchmark:gc.alloc.rate longMethod avgt 1000 0.046 ± 0.006 MB/sec
MethodProbeBenchmark.benchmark:gc.alloc.rate.norm longMethod avgt 1000 ≈ 10⁻⁴ B/op
MethodProbeBenchmark.benchmark intMethod avgt 1000 5.643 ± 0.006 ns/op
MethodProbeBenchmark.benchmark:gc.alloc.rate intMethod avgt 1000 0.046 ± 0.006 MB/sec
MethodProbeBenchmark.benchmark:gc.alloc.rate.norm intMethod avgt 1000 ≈ 10⁻⁴ B/op
MethodProbeBenchmark.benchmark longMethodObject avgt 1000 4.082 ± 0.055 ns/op
MethodProbeBenchmark.benchmark:gc.alloc.rate longMethodObject avgt 1000 0.046 ± 0.006 MB/sec
MethodProbeBenchmark.benchmark:gc.alloc.rate.norm longMethodObject avgt 1000 ≈ 10⁻⁴ B/op
MethodProbeBenchmark.benchmark integerMethodObject avgt 1000 3.833 ± 0.010 ns/op
MethodProbeBenchmark.benchmark:gc.alloc.rate integerMethodObject avgt 1000 0.046 ± 0.006 MB/sec
MethodProbeBenchmark.benchmark:gc.alloc.rate.norm integerMethodObject avgt 1000 ≈ 10⁻⁴ B/op
There's no (significant) difference in memory allocation or execution time suggesting that not having an IntProbe for example is causing an issue.
There was a problem hiding this comment.
Normally a float gets cast to a double, but we are talking about signature polymorphism here; so it isn't normal Java code.
This is a JMH benchmark that shows for both an int/float type if there is an upcast to either long/double.
package examples;
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.Scope;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Warmup;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.VarHandle;
import java.util.concurrent.TimeUnit;
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@BenchmarkMode(Mode.AverageTime)
@State(Scope.Benchmark)
@Fork(value = 2)
@Measurement(iterations = 2)
@Warmup(iterations = 1)
public class VarHandleMismatchBenchmark {
private static final VarHandle INT_VALUE;
private static final VarHandle FLOAT_VALUE ;
static {
try {
MethodHandles.Lookup l = MethodHandles.lookup();
INT_VALUE = l.findVarHandle(VarHandleMismatchBenchmark.class, "intValue", int.class);
FLOAT_VALUE = l.findVarHandle(VarHandleMismatchBenchmark.class, "floatValue", float.class);
} catch (ReflectiveOperationException e) {
throw new ExceptionInInitializerError(e);
}
}
public int intValue;
public float floatValue;
@Benchmark
public int int_ExactMatch() {
return (int) INT_VALUE.get(this);
}
@Benchmark
public long int_upcastToLong() {
return (long) INT_VALUE.get(this);
}
@Benchmark
public float float_ExactMatch() {
return (float) FLOAT_VALUE.get(this);
}
@Benchmark
public double float_upcastToDouble() {
return (double) FLOAT_VALUE.get(this);
}
}
From the performance results it is clear that an upcast isn't treated the same as an exact match.
VarHandleMismatchBenchmark.float_ExactMatch avgt 4 0.396 ± 0.349 ns/op
VarHandleMismatchBenchmark.float_upcastToDouble avgt 4 8.829 ± 1.180 ns/op
VarHandleMismatchBenchmark.int_ExactMatch avgt 4 0.396 ± 0.332 ns/op
VarHandleMismatchBenchmark.int_upcastToLong avgt 4 8.941 ± 1.271 ns/op
I have not run with a profiler, but I would not be surprised of the upcast versions suffer from litter.
There was a problem hiding this comment.
This is a JMH benchmark that shows for both an int/float type if there is an upcast to either long/double.
See my previous comment, I don't think this issue exists with my implementation using MethodHandles. But this does need to be addressed when we try to use FieldProbes with VarHandles.
pveentjer
reviewed
Sep 25, 2023
| super(field, probe, type, sourceMetadata); | ||
| } | ||
|
|
||
| @Override | ||
| public long get(S source) throws Exception { | ||
| switch (type) { | ||
| case TYPE_PRIMITIVE_LONG: | ||
| case TYPE_LONG_PRIMITIVE: |
There was a problem hiding this comment.
Once the type system is enriched with the other primitive types, you can use a VarHandle and use the same technique to prevent litter as with the MethodProbe.
There was a problem hiding this comment.
Once the type system is enriched with the other primitive types, you can use a VarHandle and use the same technique to prevent litter as with the MethodProbe.
I thought it already did for some reason.
Raised separately, although a very easy change.
There was a problem hiding this comment.
I think the type enrichment can be left for a future PR
|
The job Click to expand the log file-------------------------- ---------SUMMARY---------- -------------------------- [ERROR] COMPILATION ERROR : -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/map/MapMerkleTreeStatsTest.java:[133,11] error: ProbeCatcher is not abstract and does not override abstract method collectThrowable(MetricDescriptor,Throwable) in MetricsCollector -------------------------- ---------ERRORS----------- -------------------------- [ERROR] COMPILATION ERROR : -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/map/MapMerkleTreeStatsTest.java:[133,11] error: ProbeCatcher is not abstract and does not override abstract method collectThrowable(MetricDescriptor,Throwable) in MetricsCollector -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/map/MapMerkleTreeStatsTest.java:[156,8] error: method does not override or implement a method from a supertype -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/internal/tstore/compaction/PartitionCompactorTest.java:[319,55] error: is not abstract and does not override abstract method collectThrowable(MetricDescriptor,Throwable) in MetricsCollector -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/internal/tstore/compaction/PartitionCompactorTest.java:[359,12] error: method does not override or implement a method from a supertype -------------------------- [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.11.0:testCompile (default-testCompile) on project hazelcast-enterprise: Compilation failure: Compilation failure: -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/map/MapMerkleTreeStatsTest.java:[133,11] error: ProbeCatcher is not abstract and does not override abstract method collectThrowable(MetricDescriptor,Throwable) in MetricsCollector -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/map/MapMerkleTreeStatsTest.java:[156,8] error: method does not override or implement a method from a supertype -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/internal/tstore/compaction/PartitionCompactorTest.java:[319,55] error: is not abstract and does not override abstract method collectThrowable(MetricDescriptor,Throwable) in MetricsCollector -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/test/java/com/hazelcast/internal/tstore/compaction/PartitionCompactorTest.java:[359,12] error: method does not override or implement a method from a supertype -------------------------- [ERROR] -> [Help 1] -------------------------- [ERROR] -------------------------- [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch. -------------------------- [ERROR] Re-run Maven using the -X switch to enable full debug logging. -------------------------- [ERROR] -------------------------- [ERROR] For more information about the errors and possible solutions, please read the following articles: -------------------------- [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException -------------------------- [ERROR] -------------------------- [ERROR] After correcting the problems, you can resume the build with the command -------------------------- [ERROR] mvn -rf :hazelcast-enterprise -------------------------- |
|
run-with-ee |
pveentjer
reviewed
Sep 26, 2023
| super(method, probe, type, sourceMetadata); | ||
| } | ||
|
|
||
| @Override | ||
| public double get(S source) throws Exception { | ||
| public double get(final S source) throws Throwable { | ||
| switch (type) { | ||
| case TYPE_DOUBLE_PRIMITIVE: |
There was a problem hiding this comment.
I believe what is needed here is:
@Override
public double get(S source) throws Exception {
switch (type) {
case TYPE_PRIMITIVE_FLOAT:
return (float) METHOD_HANDLE.invokeExact(source);
case TYPE_PRIMITIVE_DOUBLE:
return (double) METHOD_HANDLE.invokeExact(source);
case TYPE_DOUBLE_NUMBER:
Number result = (Number) method.invoke(source, EMPTY_ARGS);
return result == null ? 0 : result.doubleValue();
default:
throw new IllegalStateException("Unrecognized type:" + type);
}
}```
There was a problem hiding this comment.
See previous comment.
pveentjer
reviewed
Sep 27, 2023
hazelcast/src/main/java/com/hazelcast/internal/metrics/impl/DoubleGaugeImpl.java
Outdated
Show resolved
Hide resolved
pveentjer
reviewed
Sep 27, 2023
hazelcast/src/main/java/com/hazelcast/internal/metrics/impl/LongGaugeImpl.java
Outdated
Show resolved
Hide resolved
pveentjer
reviewed
Sep 27, 2023
hazelcast/src/main/java/com/hazelcast/internal/metrics/impl/MethodProbe.java
Outdated
Show resolved
Hide resolved
pveentjer
reviewed
Sep 27, 2023
hazelcast/src/main/java/com/hazelcast/internal/metrics/impl/MetricsCollectionCycle.java
Outdated
Show resolved
Hide resolved
pveentjer
reviewed
Sep 27, 2023
hazelcast/src/main/java/com/hazelcast/internal/metrics/impl/MetricsCollectionCycle.java
Outdated
Show resolved
Hide resolved
|
The job Click to expand the log file-------------------------- ---------SUMMARY---------- -------------------------- [ERROR] COMPILATION ERROR : -------------------------- [ERROR] reason: class file for aQute.bnd.annotation.Resolution not found -------------------------- ---------ERRORS----------- -------------------------- [ERROR] COMPILATION ERROR : -------------------------- [ERROR] reason: class file for aQute.bnd.annotation.Resolution not found -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/main/java/com/hazelcast/map/impl/EnterpriseMapMigrationAwareService.java:[22,31] error: cannot find symbol -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/main/java/com/hazelcast/map/impl/EnterpriseMapMigrationAwareService.java:[170,75] error: cannot find symbol -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/main/java/com/hazelcast/internal/tstore/compaction/LocalIndexPartitionCompactorStep.java:[7,31] error: cannot find symbol -------------------------- [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.11.0:compile (default-compile) on project hazelcast-enterprise: Compilation failure: Compilation failure: -------------------------- [ERROR] reason: class file for aQute.bnd.annotation.Resolution not found -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/main/java/com/hazelcast/internal/tstore/service/impl/TieredStoreServiceImplMetrics.java:[18,31] error: cannot find symbol -------------------------- [ERROR] symbol: class Indexes -------------------------- [ERROR] location: package com.hazelcast.query.impl -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/main/java/com/hazelcast/map/impl/EnterpriseMapMigrationAwareService.java:[22,31] error: cannot find symbol -------------------------- [ERROR] symbol: class Indexes -------------------------- [ERROR] location: package com.hazelcast.query.impl -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/main/java/com/hazelcast/map/impl/EnterpriseMapMigrationAwareService.java:[170,75] error: cannot find symbol -------------------------- [ERROR] symbol: class Indexes -------------------------- [ERROR] location: class EnterpriseMapMigrationAwareService -------------------------- [ERROR] /home/jenkins/jenkins_slave/workspace/Hazelcast-pr-EE-compiler/hazelcast-enterprise/hazelcast-enterprise/src/main/java/com/hazelcast/internal/tstore/compaction/LocalIndexPartitionCompactorStep.java:[7,31] error: cannot find symbol -------------------------- [ERROR] symbol: class Indexes -------------------------- [ERROR] location: package com.hazelcast.query.impl -------------------------- [ERROR] -> [Help 1] -------------------------- [ERROR] -------------------------- [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch. -------------------------- [ERROR] Re-run Maven using the -X switch to enable full debug logging. -------------------------- [ERROR] -------------------------- [ERROR] For more information about the errors and possible solutions, please read the following articles: -------------------------- [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException -------------------------- [ERROR] -------------------------- [ERROR] After correcting the problems, you can resume the build with the command -------------------------- [ERROR] mvn -rf :hazelcast-enterprise -------------------------- |
|
run-with-ee |
|
The job Click to expand the log file---------ERRORS----------- -------------------------- [ERROR] Tests run: 17, Failures: 1, Errors: 0, Skipped: 1, Time elapsed: 196.8 s <<< FAILURE! -- in com.hazelcast.jet.kinesis.KinesisIntegrationTest -------------------------- [ERROR] com.hazelcast.jet.kinesis.KinesisIntegrationTest.dynamicStream_1Shard_splitsDuringData -- Time elapsed: 126.7 s <<< FAILURE! -------------------------- [ERROR] Failures: -------------------------- [ERROR] KinesisIntegrationTest.dynamicStream_1Shard_splitsDuringData:324->dynamicStream_splitsDuringData:354->assertMessages:637->AbstractKinesisTest.assertMessages:163->HazelcastTestSupport.assertTrueEventually:1269->HazelcastTestSupport.assertTrueEventually:1165->AbstractKinesisTest.lambda$assertMessages$3:180 Messages for key 241 differ! -------------------------- [ERROR] Tests run: 26, Failures: 1, Errors: 0, Skipped: 3 -------------------------- [ERROR] There are test failures. -------------------------- -------------------------- -------TEST FAILURE------- -------------------------- [INFO] Results: [INFO] [ERROR] Failures: [ERROR] KinesisIntegrationTest.dynamicStream_1Shard_splitsDuringData:324->dynamicStream_splitsDuringData:354->assertMessages:637->AbstractKinesisTest.assertMessages:163->HazelcastTestSupport.assertTrueEventually:1269->HazelcastTestSupport.assertTrueEventually:1165->AbstractKinesisTest.lambda$assertMessages$3:180 Messages for key 241 differ! expected: 10 actual: 632 expected:<[241: msg 000000241, 241: msg 000000491, 241: msg 000000741, 241: msg 000000991, 241: msg 000001241, 241: msg 000001491, 241: msg 000001741, 241: msg 000001991, 241: msg 000002241, 241: msg 000002491]> but was:<[241: msg 000000241, 241: msg 000000491, 241: msg 000000741, 241: msg 000000991, 241: msg 000001241, 241: msg 000001491, 241: msg 000001741, 241: msg 000001991, 241: msg 000002241, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491, 241: msg 000002491]> [INFO] [ERROR] Tests run: 26, Failures: 1, Errors: 0, Skipped: 3 [INFO] |
|
run-with-ee |
vbekiaris
approved these changes
Oct 2, 2023
There was a problem hiding this comment.
the PR looks good, thanks @pveentjer for the comments & @JackPGreen for the improvements.
About finals: I think they add more noise than convey important info, especially marking exceptions in catch blocks or method arguments in one-liners is superfluous. I'd rather see them gone.
|
run-with-ee |
pveentjer
reviewed
Oct 2, 2023
| } | ||
|
|
||
| for (final Class<?> interfaze : clazz.getInterfaces()) { | ||
| result.add(interfaze); |
There was a problem hiding this comment.
What about duplicates? The 'result' class is a list and will not provide any protection against duplicates.
There was a problem hiding this comment.
What about duplicates? The 'result' class is a list and will not provide any protection against duplicates.
result was an ArrayList but I've changed to a LinkedHashSet. I don't think duplicates had been considered in the design.
pveentjer
reviewed
Oct 2, 2023
| if (!result.contains(clazz)) { | ||
| result.add(clazz); | ||
| } | ||
| static void flatten(final Class<?> clazz, final Collection<Class<?>> result) { |
There was a problem hiding this comment.
Why not make this a Set instead of a Collection? Then it is obvious no duplicate checking is needed.
There was a problem hiding this comment.
Why not make this a Set instead of a Collection? Then it is obvious no duplicate checking is needed.
I could've done, but I prefer using more abstract types if possible.
The callers only ever use an Iterator and contains on the returned collection, so offering a more abstract type allows internal refactoring without affecting callers.
pveentjer
reviewed
Oct 2, 2023
| assertFalse(ProbeUtils.isDouble(TYPE_MAP)); | ||
| assertFalse(ProbeUtils.isDouble(TYPE_COUNTER)); | ||
| /** Prints the types that a @Probe has been attached to in the codebase, and where they can be found */ | ||
| public static void main(final String[] args) { |
There was a problem hiding this comment.
Perhaps not hide this function in a test?
There was a problem hiding this comment.
Perhaps not hide this function in a test?
Where would you suggest putting it instead? It's a development tool that might be useful later, but not something we want to deploy with the product.
|
Re-ran benchmarks after PR changes, on a different PC so absolute times have changed but still an improvement is shown. |
pveentjer
approved these changes
Oct 3, 2023
|
Can't wait to have this in master :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When accessing the data provided by
MethodProbes, currently core Java reflection is used.I've replaced this with
MethodHandles:MethodHandleit's possible to directly access a primitive, without boxing/unboxing - reducing object creation (and therefore garbage collection overhead)Benchmarks:
I benchmarked (
MethodProbeBenchmark) the improvement made to a the usage of aMethodProbe:longprimitiveMethodHandlelongprimitiveLongObjectMethodHandleLongObjectThe improvements are:
This benchmark includes the overhead of the method operation itself - so is reflective of the real-world observable performance improvement, rather than a theoretical comparison of the two implementations.
More information on the implementation can be found in the Javadoc of
MethodProbe.Summary of Changes
MethodProbeuseMethodHandlesintconstants inProbeUtilstoProbeTypeenumflattening logic to avoid aList.containsiterationFieldProbeto suit refactoringFixes #25244