Remove some lambda expressions

[geoand]

These are causing NoSuchMethodError on application startup (this can be verified easily either via the debugger, or from JFR events - see https://bugs.openjdk.java.net/browse/JDK-8161588 for more details about why the error occurs) which can have a small impact on startup performance

N.B. I only replaced the code for the instances that were causing the issue at the startup of a Quarkus application that uses Vert.x

[tsegismont]

@geoand out of curiosity, what's the Quarkus related issue?

[geoand]

It's not a functionality issue per-se, it's more of a minor performance nussance.

Essentially what happens is that the as part of the way the JDK implements some lambdas and method handles, it throws a NoSuchMethodError (any exception throwing is not good for performance) and also records a JFR event for it (which can send users on a wild goose chace - both myself and @franz1981 have been down this road...)

[tsegismont]

Is there any way to understand why these two in particular are a problem? At least we should have a comment explaining why we must have an anonymous class instead of a lambda.

Do you know if this problem will happen for every call to runOnContext? I'm asking because it's a very common construct in vert.x internally and in vert.x apps.

[geoand]

Is there any way to understand why these two in particular are a problem

I personally have not weeded through the JDK code that leads to this. Maybe @franz1981 has?

Do you know if this problem will happen for every call to runOnContext

Not sure. If you have samples, I can try them and we can see in practice.

[franz1981]

@tsegismont

See https://bugs.openjdk.java.net/browse/JDK-8161588?focusedCommentId=13974951&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-13974951 for more info The TLDR story is that some method handle (missing field) lookups using lambda can stealthy throws exceptions: so, it won't affect hot code (because the lookup will fail the first time, then will be likely cached somewhere) and it affect where there's some reflective lookup in place IIUC (it's our case @geoand ?)

Do you know if this problem will happen for every call to runOnContext?

Nope, just ones used to setup/config things/on startup, the first time ie cold paths by definition, but still important to reduce startup time :) I still suggest to use lambda elsewhere, int the hot path, because lambdas got the enviable property of having "truly and trusted" final fields of the captured context, while this won't happen with anonymous classes IIRC (and are cached automatically, if lucky :P )

[geoand]

and it affect where there's some reflective lookup in place IIUC (it's our case @geoand ?)

No reflective access in our case

[vietj]

I'm not in favour of dropping lambdas, unless that is an important performance hit.

can you provide benchmark that exhibit this ?

[geoand]

The performance hit is not really measurable. It only makes a small difference when there are a lot NSME thrown (as explained in the description) at application startup. We are trying to cut these down in Quarkus

[geoand]

Just to get back to this, here is what the flamegraph for the NoSuchMethodError exceptions thrown during startup looks like:

This change would remove most of them (my JFR sample has 108 of these exceptions where the majority are caused by lambdas in Vert.x)

[geoand]

I should also mention that JMC flags these exceptions as Max Severity in its analysis of the JFR recording.