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

Use method references instead of reflection in TypeUtil #9115

Merged

Conversation

kohlschuetter
Copy link
Contributor

TypeUtil currently uses reflection to convert Strings to primitive/boxed basic types.

This may not only be a performance problem, but it also prevents environments like GraalVM native-image to detect required methods without the help of an agent.

Use method references (Boolean::valueOf, etc.) instead.

TypeUtil currently uses reflection to convert Strings to primitive/boxed
basic types.

This may not only be a performance problem, but it also prevents
environments like GraalVM native-image to detect required methods
without the help of an agent.

Use method references (Boolean::valueOf, etc.) instead.

Signed-off-by: Christian Kohlschütter <[email protected]>

@FunctionalInterface
private interface ValueOfString<T> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this just be Function<String, T>? It's all internal anyway, no need for another type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea; see commit below

... and use Function<String,Object> instead.
@sbordet sbordet self-requested a review January 2, 2023 00:27
@joakime
Copy link
Contributor

joakime commented Jan 2, 2023

Somehow this PR has an enforcer rule violation ????

[INFO] --- maven-enforcer-plugin:3.1.0:enforce (enforce-java) @ jetty-infinispan-embedded ---
[ERROR] Rule 4: org.apache.maven.plugins.enforcer.RequireUpperBoundDeps failed with message:
Failed while enforcing RequireUpperBoundDeps. The error(s) are [
Require upper bound dependencies error for com.fasterxml.jackson.core:jackson-core:2.13.4 paths to dependency are:
+-org.eclipse.jetty:jetty-infinispan-embedded:12.0.0-SNAPSHOT
  +-org.infinispan:infinispan-core:11.0.16.Final
    +-org.infinispan.protostream:protostream:4.6.0.Final (managed) <-- org.infinispan.protostream:protostream:4.3.4.Final
      +-com.fasterxml.jackson.core:jackson-core:2.13.4 (managed) <-- com.fasterxml.jackson.core:jackson-core:2.14.1
]

I'll dig into this in the morning.

@sbordet
Copy link
Contributor

sbordet commented Jan 2, 2023

@joakime I think it's an issue solved by @olamy in 3d7ccb1

@joakime
Copy link
Contributor

joakime commented Jan 2, 2023

@kohlschuetter can you merge from jetty-12.0.x to this branch? (hopefully it will get a green build)

@joakime joakime added this to the 12.0.x milestone Jan 2, 2023
@joakime joakime merged commit 360bd2e into jetty:jetty-12.0.x Jan 2, 2023
@joakime
Copy link
Contributor

joakime commented Jan 2, 2023

Looking good, merging.

@joakime
Copy link
Contributor

joakime commented Jan 9, 2023

@gregw should we backport this to Jetty 10.0.x?

@joakime joakime changed the title core: util: TypeUtil: Use method references instead of reflection Use method references instead of reflection in TypeUtil Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants