Copilot commented on code in PR #12750:
URL: https://github.com/apache/gluten/pull/12750#discussion_r3755303129
##########
gluten-core/src/main/scala/org/apache/gluten/extension/columnar/transition/TransitionGraph.scala:
##########
@@ -130,8 +136,13 @@ object TransitionGraph {
if (diff != 0) {
diff
} else {
- // To make the output order stable.
- nodeNames1.mkString.hashCode - nodeNames2.mkString.hashCode
+ // Break the tie on the node names so the chosen path is stable
across JVM runs.
+ // Integer.compare rather than a subtraction: hash codes far
enough apart overflow
+ // Int, and a wrapped sign makes FloydWarshallGraph replace the
incumbent path with a
+ // more expensive one. Note two distinct name sequences can
still share a hash code,
+ // and mkString joins without a separator, so ties are possible;
the winner then comes
+ // down to map iteration order, as it did before.
Review Comment:
The updated comment says the overflowed subtraction "makes
FloydWarshallGraph replace the incumbent path with a more expensive one", but
this tie-breaker only runs when `diff == 0` (equal base costs). The overflow
can flip which *equal-cost* path wins the tie, not select a higher-cost path.
Also, "stable across JVM runs" is not always true due to possible hash/mkString
collisions (which you already mention below), so the first sentence should be
softened to avoid overpromising determinism.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
