-
Notifications
You must be signed in to change notification settings - Fork 3k
Introduce LambdaCapturingTypeBuildItem #25861
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
It's marked as draft until a test is available 😉 |
8a3ad3c to
16df728
Compare
|
I've been also thinking of adding a new |
galderz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far it looks ok to me.
zakkak
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as well. Thanks @gastaldi
d1b5210 to
99c586d
Compare
|
@zhfeng that is really weird indeed. I debugged and the code produced in the generated Any ideas? |
fab1eb8 to
59a8280
Compare
|
@gastaldi I have no idea but I just tried to get an Exception when using I hope it is helpful! |
|
I think our handle It could be different with graalvm < 22.1 ? |
I saw this same exception during my tests, that's why I wrapped the calls in a try-catch block, it sounds like a bug in GraalVM.
That same method is called when the
I don't know if serializing lambdas were supported in pre-22.1 versions, but I wouldn't hold my breath, since Quarkus recommends 22.1 :) |
|
@gastaldi Hmm, it looks like |
|
@gastaldi by using but by using |
|
That's a good point. I get different results but it looks like the same 12 methods are missing. With With I wonder which methods are these |
|
@zhfeng these are the missing methods: private final java.lang.Object java.util.Comparator$$Lambda$283/0x00000003025fcf50.writeReplace()
private final java.lang.Object java.util.Comparator$$Lambda$478/0x0000000302670a00.writeReplace()
private final java.lang.Object java.util.Comparator$$Lambda$479/0x0000000302670ca0.writeReplace()
private final java.lang.Object java.util.Comparator$$Lambda$480/0x0000000302670f40.writeReplace()
private final java.lang.Object java.util.Comparator$$Lambda$481/0x0000000302671470.writeReplace()
private final java.lang.Object java.util.Comparator$$Lambda$482/0x0000000302671710.writeReplace()
private final java.lang.Object java.util.Comparator$$Lambda$483/0x00000003026719b0.writeReplace()
private final java.lang.Object java.util.Comparator$$Lambda$484/0x0000000302671c50.writeReplace()
private final java.lang.Object java.util.Comparator$$Lambda$485/0x0000000302671ef0.writeReplace()
private final java.lang.Object java.util.Comparator$$Lambda$486/0x0000000302672190.writeReplace()
private final java.lang.Object java.util.Comparator$$Lambda$80/0x00000003000b0bc8.writeReplace()
private final java.lang.Object java.util.Comparator$$Lambda$85/0x00000003000b1888.writeReplace() |
|
@gastaldi nice investigating! - how did you get them? And I think that could be a reason for |
I created another @AutomaticFeature
public class GetMethodsAutoFeature implements Feature {
@Override
public void afterAnalysis(AfterAnalysisAccess access) {
RuntimeReflectionSupport support = ImageSingletons.lookup(RuntimeReflectionSupport.class);
try {
try (OutputStream os = Files
.newOutputStream(Path.of("/Users/ggastald/workspace/quarkus/integration-tests/main/count.txt"));
PrintStream ps = new PrintStream(os, true)) {
support.getReflectionExecutables().stream().map(Executable::toString)
.sorted(String.CASE_INSENSITIVE_ORDER)
.forEach(ex -> ps.println(ex));
}
} catch (IOException e) {
throw new RuntimeException(e);
}
}
}And compared the generated files on each run :)
Probably, still not sure what's missing in the generated bytecode |
|
@galderz It looks like we register see |
|
Is there any way to let |
|
@gastaldi can we register the lambda capuring class at here? |
|
@gastaldi it works with |
|
The reason is that we have to register these |
* do lambda capuring class in duringSetup * update ResourceLambda to use ByteArrayOutputStream * remove serialization-config.json
40b8657 to
a1909e9
Compare
|
Nice catch @zhfeng ! |
This comment has been minimized.
This comment has been minimized.
|
@zhfeng I don't know how GraalVM orders the But does it really matter now that you moved the registration to |
|
@gastaldi I think we are using it to register the lambdaCapuringClass. |
TLDR: @zhfeng
TLDR: @gastaldi yes it still matters. Hi @zhfeng and @gastaldi I will try to summarize what's happening and please correct me if I misunderstood something.
Since there is no other "handler" to run between public List<Class<? extends Feature>> getRequiredFeatures() {
return Collections.singletonList(SerializationFeature.class);
}Which we already do in quarkus/core/deployment/src/main/java/io/quarkus/deployment/steps/NativeImageAutoFeatureStep.java Lines 537 to 548 in b32d4fc
(note that adding HTH |
|
Thanks a lot @zakkak and it is much more clear now! |
|
I think it should be good to add this new feature in the next Quarkus release. Is there anything we need to do before merging? |
gsmet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice stuff. Let's get it in and we can tweak it later if needed.
This introduces support for the
lambdaCapturingTypeconfiguration introduced in oracle/graal@b455f23Fixes #24861