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

[GR-60235] Register reflectively-accessed types as unsafe allocated #10214

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

graalvmbot
Copy link
Collaborator

All reflectively-accessible classes are now registered for unsafe allocation. This removes "unsafeAllocated" from reachability-metadata.json, and was evaluated to have only a minimal impact on the resulting image size (see #9679). In effect, the following JSON registration:

{
  "reflection": [
    {
      "type": "registered.class.Name",
    }
  ]
}

will become equivalent to:

{
  "reflection": [
    {
      "type": "registered.class.Name",
      "unsafeAllocated": true
    }
  ]
}

In the same fashion, calling RuntimeReflection.register(clazz) from a feature will make clazz unsafe-allocatable at run-time.

Existing JSON metadata files using the flag will continue to be accepted by Native Image, with a warning printed once per build.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Dec 3, 2024
@loicottet loicottet self-assigned this Dec 3, 2024
@zakkak
Copy link
Collaborator

zakkak commented Dec 3, 2024

This removes "unsafeAllocated" from reachability-metadata.json, and was evaluated to have only a minimal impact on the resulting image size (see #9679).

@loicottet as mentioned in #9679 (comment)

The maximum size impact (3%) on treating all types as unsafe allocated seems quite high.

As a result, we would still like to have the option to opt-out of this (i.e. to set unsafeAllocated to false). Could you please keep this functionality? I believe (in general) that it would be better to just switch the default instead of removing the options.

@graalvmbot graalvmbot force-pushed the lottet/GR-60235-default-unsafe-allocate branch 2 times, most recently from c6771bc to f6d7b16 Compare December 4, 2024 09:16
@zakkak
Copy link
Collaborator

zakkak commented Dec 5, 2024

FWIW, after evaluating this PR it seems that it adds more overhead than the registration of all fields (in the integration tests of Quarkus core). See quarkusio/quarkus#44864 (comment) for more details.

@graalvmbot graalvmbot force-pushed the lottet/GR-60235-default-unsafe-allocate branch 5 times, most recently from 20f8d5e to 41a074b Compare December 11, 2024 14:58
@graalvmbot graalvmbot force-pushed the lottet/GR-60235-default-unsafe-allocate branch from 41a074b to 700d84a Compare December 13, 2024 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants