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

ClassUtils optimization for less expensive static initialization [SPR-17169] #21705

Closed
spring-issuemaster opened this issue Aug 12, 2018 · 4 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Aug 12, 2018

Dave Syer opened SPR-17169 and commented

Spring core ClassUtils is always an annoying blip on the flame
graphs of startup benchmarks. It pops up because it indirectly calls
java.lang.invoke.MethodHandleNatives.linkCallSite() which is
expensive it seems, or at least when called in a static
initializer. You can avoid the cost completely if you switch from
iterating over a map using a lambda to explicit old-style iteration:

for (Map.Entry<Class<?>, Class<?>> entry : primitiveWrapperTypeMap.entrySet()) {
     Class<?> key = entry.getKey();
     Class<?> value = entry.getValue();
     primitiveTypeToWrapperMap.put(value, key);
     registerCommonClasses(key);
}

instead of

primitiveWrapperTypeMap.forEach((key, value) -> {
     primitiveTypeToWrapperMap.put(value, key);
     registerCommonClasses(key);
});

The improvement in micro apps startup is dramatic: about 3.5%.


Affects: 5.0.8

Referenced from: commits df51ff0, 1d8e5f4

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 12, 2018

Juergen Hoeller commented

If that makes a significant difference, the JVM has some room to improve...

Anyway, I've adapted the static initializer along those lines, effectively downgrading the code to its 4.3.x version again.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 12, 2018

Dave Syer commented

Agree the JVM probably has a problem. Unfortunately it doesn’t look like it’s one that we can work around simply. My benchmarks don’t show a commensurate improvement in startup time after this change - there is some improvement but not as dramatic as I had hoped. But, the flame graph no longer highlights ClassUtils as a hot spot. So we have achieved something, and this change didn’t cost much, so it’s worth keeping. We probably just pushed the actual issue into another place, so I’ll keep digging.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 13, 2018

Dave Syer commented

I looked at the flame graphs again, before and after. As feared, this change seems to just shuffle the problem around. The same work ends up being done in the JVM somewhere else - e.g. in one app I saw the same ~3.5% penalty for MethodHandleNatives.linkCallSite() but coming from LoggingSystem.get() in Spring Boot, whereas before this change that flame wasn't visible.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 13, 2018

Juergen Hoeller commented

No worries. Since this is just a syntactic change with hardly a drawback, I'll keep it that way in ClassUtils, not triggering any particular initialization impact on our end (and preserving the initialization behavior that we had in 4.3.x already).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.