Make use of eq and ne.#1994
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes performance by replacing ==/!= equality comparisons with reference equality checks (eq/ne) when comparing against null. Reference equality is more performant than value equality as it only checks if two references point to the same object in memory, rather than potentially calling the equals method.
- Replaces
== nullwitheq nullthroughout the codebase - Replaces
!= nullwithne nullthroughout the codebase - Adds parentheses around compound conditions for clarity when needed
Reviewed Changes
Copilot reviewed 84 out of 84 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| testkit/src/main/scala/org/apache/pekko/testkit/javadsl/EventFilter.scala | Updates null comparison in exception type check |
| testkit/src/main/scala/org/apache/pekko/testkit/TestKit.scala | Updates null comparison in queue polling logic |
| stream/src/main/scala/org/apache/pekko/stream/stage/GraphStage.scala | Updates multiple null comparisons in graph stage logic |
| stream/src/main/scala/org/apache/pekko/stream/scaladsl/Hub.scala | Updates null comparison in consumer management |
| stream/src/main/scala/org/apache/pekko/stream/scaladsl/Graph.scala | Updates null comparisons in merge operations |
| stream/src/main/scala/org/apache/pekko/stream/javadsl/* | Updates null comparisons in Java DSL classes |
| stream/src/main/scala/org/apache/pekko/stream/impl/* | Updates null comparisons in stream implementation classes |
| serialization-jackson/src/main/scala/org/apache/pekko/serialization/jackson/JacksonSerializer.scala | Updates null comparisons in serialization logic |
| remote/src/main/scala/org/apache/pekko/remote/* | Updates null comparisons in remote transport and serialization |
| persistence/src/main/scala/org/apache/pekko/persistence/* | Updates null comparisons in persistence logic |
| cluster*/src/main/scala/org/apache/pekko/cluster/* | Updates null comparisons in cluster components |
| actor*/src/main/scala/org/apache/pekko/actor/* | Updates null comparisons in actor system components |
| case Spawn(bhvr: Behavior[t], name, props, replyTo) => | ||
| val ref = | ||
| if (name == null || name.equals("")) | ||
| if ((name eq null) || name.equals("")) |
There was a problem hiding this comment.
are the extra parentheses needed?
There was a problem hiding this comment.
likewise for all the extra parentheses?
There was a problem hiding this comment.
Yes, otherwise compiling error
There was a problem hiding this comment.
This looks good since we are only using ne/eq in null comparison checks which is the only safe case to use it.
Merge after you resolve @pjfanning 's comments regarding extra parens
This can also be backported to the 1.2.x series as its safe to do so (even 1.1.x if we are so inclined)
|
@He-Pin Can you make backported/cherry picked commits for 1.2.x and 1.1.x? |
|
yes, but not sure @pjfanning can agree |
|
Motivation:
eq and ne are more performance