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

Spring Performance Optimization, Comparing Classes [SPR-12926] #17519

Closed
spring-issuemaster opened this issue Apr 16, 2015 · 6 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Apr 16, 2015

Daniel P Rossi opened SPR-12926 and commented

This is a task to update class comparisons to use "==" instead of ".equals()". There is a significant performance gain in this change. It also makes utilities like "BeanPropertyRowMapper" just as efficient as writing a custom row mapper.


Reference URL: #773

Attachments:

Issue Links:

  • #19493 Improve performance for conversions using a method parameter based type descriptor with annotations
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 7, 2015

Juergen Hoeller commented

On review, it seems odd that there is a significant performance gain here: java.lang.Class doesn't seem to override Object.equals and therefore inherits a straight reference equality check from it. In a live JVM, that reference comparison would get inlined and should be effectively identical to a direct reference check. Could you provide some details on why this makes a significant difference in your scenario?

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 18, 2015

Daniel P Rossi commented

I've written and attached a test that demonstrates the performance gain with "JdbcUtils.getResultSetValue" being changed to reference checks instead of equal checks...

Before: 42ms, 1000 tests, 14 distinct values
After: 18ms, 1000 tests, 14 distinct values

This test should help prove my point. Let me know if there are any further questions.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 18, 2015

Juergen Hoeller commented

In such a micro-benchmark, the JVM isn't going to inline those calls yet. Method call overhead is going to make a significant difference there, in particular against a dummy ResultSet implementation. In a warmed-up JVM, those equals calls should all be inlined and the resulting performance equivalent to direct reference comparisons.

We - and I'd argue plenty of developers - rely on inlining for efficiency at runtime. Otherwise, even Assert calls or calls to utility methods would be inefficient. So generally speaking, optimizing a case that would be inlined anyway falls into the category of "premature optimization" in source code.

Also, the Java language specification does not explicitly guarantee that Class can be compared by reference. Class.equals may theoretically be implemented in a more lenient form on some JVMs, and we need to make sure we're correctly handling such comparisons in the general case.

Now, that said, I wouldn't mind optimizing the checks in JdbcUtils since they're all referring to built-in JVM types which I can't imagine to be present in a non-unique form. It's just in other places in the framework where we're comparing user types for which I'd stick with equals for defensiveness.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 18, 2015

Daniel P Rossi commented

I think having only JdbcUtils changed for this would be just fine (that was all I was originally after). It has a significant performance increase when using BeanPropertyRowMapper. This makes the need to write custom row mappers almost obsolete and can increase development time where this class becomes useful.

I also agree defensive programming is a better way to go. Although, if there are any other places where multiple class types are being checked (like JdbcUtils), then I would strongly recommend this change as well.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 19, 2015

Juergen Hoeller commented

I'm doing a broader sweep through the codebase now where I'm replacing all comparisons with hard-coded Class references (i.e. X.class.equals) with identity comparisons. Also, we can use IdentityHashMap for maps that only hold built-in types in keys anyway (such as in ClassUtils and BeanUtils).

In other words, we're relying on Class identity for JDK types and Spring interface types now. This should be a safe assumption since otherwise we couldn't refer to those types in our source files or cast to them either. At the same time, we keep using Class.equals for any comparisons between user types.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 19, 2015

Juergen Hoeller commented

This turns out to be a pretty worthwhile sweep, reviewing quite a few hotspots. NumberUtils does plenty of class comparisons as well and may get invoked for many properties per request. The same applies to various parts of our dependency injection resolution algorithm.

The principle of comparing JDK classes and Spring interfaces by identity is now consistently applied across the codebase. We'll see whether there are any subtle regressions in the 4.2 RC phase but I doubt that there can be regressions for those specific purposes.

Juergen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.