Skip to content
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

Extend ReflectiveClassBuildItem to support queryOnly option #42035

Merged

Conversation

zakkak
Copy link
Contributor

@zakkak zakkak commented Jul 22, 2024

queryOnly registrations enable us to register only the metadata without the actual code, essentially reducing the native executable size. For instance, if some code invokes getDeclaredMethods on a class to see if a specific method is available we don't actually need all the methods bundled in the native executable, we just need their metadata. So we could use queryAllMethods instead of methods (which will also pull in the code of all methods).

Note that for the time being we only extend it to support
queryAllDeclaredConstructors and queryAllMethods since we don't have an
option to register only public methods.

Closes #41999

@zakkak zakkak marked this pull request as ready for review July 22, 2024 12:31

This comment has been minimized.

@zakkak zakkak requested review from gsmet and geoand July 23, 2024 07:48
@geoand
Copy link
Contributor

geoand commented Jul 23, 2024

Can you please update the description of the PR and add some Javadoc in the code to describe what exactly GR-21937 brings to the table?
Reading the description of #41999 (and its associated links), I am unfortunately none the wiser about what this does and when I should use it.

Thanks

@zakkak zakkak force-pushed the 2024-07-22-extend-RedlectiveClassBuildItem branch from a108468 to a087e59 Compare July 23, 2024 12:11
@zakkak
Copy link
Contributor Author

zakkak commented Jul 23, 2024

@geoand done, please have another look and let me know if it's clear now.

queryOnly registrations enables us to register only the metadata without
the actual code, essentially reducing the native executable size.  For
instance, if some code invokes getDeclaredMethods on a class to see if a
specific method is available we don't actually need all the methods
bundled in the native executable, so we could use queryAllMethods
instead of methods (which will also pull in the code of all methods).

Note that for the time being we only extend it to support
queryAllDeclaredConstructors and queryAllMethods since we don't have an
option to register only public methods.

Closes quarkusio#41999
@zakkak zakkak force-pushed the 2024-07-22-extend-RedlectiveClassBuildItem branch from a087e59 to 082269d Compare July 23, 2024 12:13
@geoand
Copy link
Contributor

geoand commented Jul 23, 2024

Very nice, thanks!

Copy link

quarkus-bot bot commented Jul 23, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 082269d.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 extensions/micrometer/deployment

io.quarkus.micrometer.deployment.binder.VertxTcpMetricsNoClientMetricsTest.testTcpMetricsWithoutClientMetrics - History

  • event executor terminated - java.util.concurrent.RejectedExecutionException
java.util.concurrent.RejectedExecutionException: event executor terminated
	at io.netty.util.concurrent.SingleThreadEventExecutor.reject(SingleThreadEventExecutor.java:931)
	at io.netty.util.concurrent.SingleThreadEventExecutor.offerTask(SingleThreadEventExecutor.java:350)
	at io.netty.util.concurrent.SingleThreadEventExecutor.addTask(SingleThreadEventExecutor.java:343)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:833)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute0(SingleThreadEventExecutor.java:824)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:814)
	at io.vertx.core.impl.EventLoopExecutor.execute(EventLoopExecutor.java:35)

@zakkak zakkak merged commit 52130c8 into quarkusio:main Jul 23, 2024
52 checks passed
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Jul 23, 2024
@zakkak zakkak deleted the 2024-07-22-extend-RedlectiveClassBuildItem branch July 23, 2024 21:17
@quarkus-bot quarkus-bot bot added this to the 3.14 - main milestone Jul 23, 2024
zakkak added a commit to zakkak/quarkus that referenced this pull request Aug 1, 2024
Fix constructors and queryConstructors check in ReflectiveClassBuildItem

Follow up to quarkusio#42035
zakkak added a commit to zakkak/quarkus that referenced this pull request Aug 1, 2024
Fix constructors and queryConstructors check in ReflectiveClassBuildItem

Follow up to quarkusio#42035
zakkak added a commit to zakkak/quarkus that referenced this pull request Aug 1, 2024
Fix constructors and queryConstructors check in ReflectiveClassBuildItem

Follow up to quarkusio#42035
zakkak added a commit to zakkak/quarkus that referenced this pull request Aug 1, 2024
Configures whether declared methods should be registered for reflection,
for query purposes only, i.e. {@link Class#getMethods()}. Setting this
enables getting all declared methods for the class but does not allow
invoking them reflectively.

Follow up to quarkusio#42035
zakkak added a commit to zakkak/quarkus that referenced this pull request Aug 1, 2024
Configures whether declared methods should be registered for reflection,
for query purposes only, i.e. {@link Class#getMethods()}. Setting this
enables getting all declared methods for the class but does not allow
invoking them reflectively.

Follow up to quarkusio#42035
zakkak added a commit to zakkak/quarkus that referenced this pull request Aug 2, 2024
Configures whether declared methods should be registered for reflection,
for query purposes only, i.e. {@link Class#getMethods()}. Setting this
enables getting all declared methods for the class but does not allow
invoking them reflectively.

Follow up to quarkusio#42035
zakkak added a commit to zakkak/quarkus that referenced this pull request Sep 4, 2024
Configures whether declared methods should be registered for reflection,
for query purposes only, i.e. {@link Class#getMethods()}. Setting this
enables getting all declared methods for the class but does not allow
invoking them reflectively.

Follow up to quarkusio#42035
danielsoro pushed a commit to danielsoro/quarkus that referenced this pull request Sep 20, 2024
Fix constructors and queryConstructors check in ReflectiveClassBuildItem

Follow up to quarkusio#42035
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants