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

Inconsistent configuration between build time and run time initialized classes of netty #42945

Open
zakkak opened this issue Sep 2, 2024 · 4 comments
Labels

Comments

@zakkak
Copy link
Contributor

zakkak commented Sep 2, 2024

Describe the bug

In #37347 we marked PlatformDependent for runtime (re)initialization. However there are still places were the (re)initialization has no effect because of classes using PlatformDependent in their static initializers being build time initialized. E.g.:

This results in some parts of the code assuming something is always true while others depend on runtime configuration.

Expected behavior

All places in code should follow the same assumptions.

Actual behavior

Some parts of the code assume something is always true (or false) while others depend on runtime configuration to figure out the actual value.

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

Mandrel or GraalVM version (if different from Java)

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

Detected while investigating #42903

Copy link

quarkus-bot bot commented Sep 2, 2024

/cc @Karm (mandrel), @cescoffier (netty), @franz1981 (netty), @galderz (mandrel), @jponge (netty), @radcortez (config)

@github-project-automation github-project-automation bot moved this to Under discussion in WG - Quarkus 3.15 LTS Sep 5, 2024
@cescoffier cescoffier moved this from Under discussion to Out of scope in WG - Quarkus 3.15 LTS Sep 5, 2024
@cescoffier
Copy link
Member

@zakkak would marking these classes as re-initialized make sense? We do not want to limit too much our ability to pre-initialize at build time, but this would be reasonable. WDYT?

@zakkak
Copy link
Contributor Author

zakkak commented Nov 21, 2024

@zakkak would marking these classes as re-initialized make sense?

Yes, that would resolve the issue if we really want to allow runtime configuration of Netty. If instead we want to limit netty configuration at build-time then we need to find another way to make the values consistent.

We do not want to limit too much our ability to pre-initialize at build time, but this would be reasonable. WDYT?

It depends on what we consider reasonable. The build-time initialization of some of these classes may be resulting in optimized code generation due to better dead code elimination (e.g. whether unsafe is available or not is decided at build-time). Similarly some static initializers of these classes are not trivial so they might add a bit of overhead at startup time. The impact of the former is hard to measure without some "targeted" benchmarsk, while the latter is easier to measure.

@cescoffier
Copy link
Member

Similarly some static initializers of these classes are not trivial so they might add a bit of overhead at startup time.

Yeah, that's what I'm seeing. Unfortunately, the only way to determine whether it's "reasonable" is to do it and measure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Out of scope
Development

No branches or pull requests

3 participants