-
Notifications
You must be signed in to change notification settings - Fork 3k
Qute: gizmo2 rewrite #49397
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
Qute: gizmo2 rewrite #49397
Conversation
0cb2181
to
cec77a7
Compare
This comment has been minimized.
This comment has been minimized.
We need to wait for Gizmo2 Beta5 that contains quarkusio/gizmo#475. |
Both Maven Tests CI jobs failed with java.lang.OutOfMemoryError: Metaspace. @gsmet is it a known issue? |
I have also seen OOM: Metaspace when testing the full Quarkus with rewritten ArC and it feels like it could be caused by Gizmo. I have no idea where to start looking though. |
Given you have We might have been just under the limit before though. |
We have |
This comment has been minimized.
This comment has been minimized.
FTR it's a ClassLoader leak caused by the usage of |
This comment has been minimized.
This comment has been minimized.
@radcortez this test |
Strange... I'll have a look. |
extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
Show resolved
Hide resolved
extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
Outdated
Show resolved
Hide resolved
extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
Outdated
Show resolved
Hide resolved
extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
Outdated
Show resolved
Hide resolved
extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
Show resolved
Hide resolved
extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
Show resolved
Hide resolved
extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
Show resolved
Hide resolved
extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
Outdated
Show resolved
Hide resolved
extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
Outdated
Show resolved
Hide resolved
extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
Outdated
Show resolved
Hide resolved
extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
Outdated
Show resolved
Hide resolved
...ndent-projects/qute/generator/src/main/java/io/quarkus/qute/generator/AbstractGenerator.java
Outdated
Show resolved
Hide resolved
...rojects/qute/generator/src/main/java/io/quarkus/qute/generator/ExtensionMethodGenerator.java
Outdated
Show resolved
Hide resolved
...rojects/qute/generator/src/main/java/io/quarkus/qute/generator/ExtensionMethodGenerator.java
Outdated
Show resolved
Hide resolved
...rojects/qute/generator/src/main/java/io/quarkus/qute/generator/ExtensionMethodGenerator.java
Outdated
Show resolved
Hide resolved
...rojects/qute/generator/src/main/java/io/quarkus/qute/generator/ExtensionMethodGenerator.java
Outdated
Show resolved
Hide resolved
...rojects/qute/generator/src/main/java/io/quarkus/qute/generator/ExtensionMethodGenerator.java
Outdated
Show resolved
Hide resolved
...rojects/qute/generator/src/main/java/io/quarkus/qute/generator/ExtensionMethodGenerator.java
Outdated
Show resolved
Hide resolved
...rojects/qute/generator/src/main/java/io/quarkus/qute/generator/ExtensionMethodGenerator.java
Outdated
Show resolved
Hide resolved
...rojects/qute/generator/src/main/java/io/quarkus/qute/generator/ExtensionMethodGenerator.java
Outdated
Show resolved
Hide resolved
...projects/qute/generator/src/main/java/io/quarkus/qute/generator/TemplateGlobalGenerator.java
Outdated
Show resolved
Hide resolved
...-projects/qute/generator/src/main/java/io/quarkus/qute/generator/ValueResolverGenerator.java
Outdated
Show resolved
Hide resolved
...-projects/qute/generator/src/main/java/io/quarkus/qute/generator/ValueResolverGenerator.java
Outdated
Show resolved
Hide resolved
...-projects/qute/generator/src/main/java/io/quarkus/qute/generator/ValueResolverGenerator.java
Outdated
Show resolved
Hide resolved
...-projects/qute/generator/src/main/java/io/quarkus/qute/generator/ValueResolverGenerator.java
Outdated
Show resolved
Hide resolved
...-projects/qute/generator/src/main/java/io/quarkus/qute/generator/ValueResolverGenerator.java
Outdated
Show resolved
Hide resolved
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. I only checked the Gizmo 1 -> 2 conversion, not the code logic. I added many comments, most of them are just my opinion, but there's nothing wrong.
I also noticed (but forgot to comment) that on may places, you create an empty array of given size and immediately fill it with given values. There's a method newArray()
that takes a list of arbitrary values and a mapping function to turn those values into Expr
s, which would turn those places into one-liners.
Nice. I actually changed the code to use the variant that accepts a list of |
Status for workflow
|
TODO