-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
HHH-14137 Empty String check performance improvements #3491
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome. I wanna repeat the article link in the ticket here: https://medium.com/javarevisited/micro-optimizations-in-java-string-equals-22be19fd8416
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have absolutely no idea if it's really worth it (microbenchmarks on this sort of things can give an incorrect picture of what you really win) but I added a couple of comments in case this goes through.
hibernate-core/src/main/java/org/hibernate/cfg/Ejb3JoinColumn.java
Outdated
Show resolved
Hide resolved
hibernate-core/src/main/java/org/hibernate/cfg/annotations/CollectionBinder.java
Outdated
Show resolved
Hide resolved
hibernate-core/src/main/java/org/hibernate/engine/internal/JoinSequence.java
Outdated
Show resolved
Hide resolved
hibernate-core/src/main/java/org/hibernate/engine/internal/JoinSequence.java
Outdated
Show resolved
Hide resolved
hibernate-core/src/main/java/org/hibernate/hql/internal/ast/util/SyntheticAndFactory.java
Outdated
Show resolved
Hide resolved
hibernate-core/src/main/java/org/hibernate/loader/OuterJoinableAssociation.java
Outdated
Show resolved
Hide resolved
hibernate-core/src/main/java/org/hibernate/loader/OuterJoinableAssociation.java
Outdated
Show resolved
Hide resolved
...n/java/org/hibernate/envers/configuration/internal/metadata/CollectionMetadataGenerator.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
Although I agree with @gsmet , this is unlikely to make any noticeable difference, but some of this code is very hot so why not.
The (minor) drawbacks I see:
- it's less readable
- if this pattern becomes "the standard way" one needs to be careful in not forgetting the null check
- we're working primarily on ORM6, we should generally not make such changes on master: please prefer the
wip/6.0branch for such changes, in case you have more changes of this type
https://hibernate.atlassian.net/browse/HHH-14137