pjfanning opened a new pull request, #3681: URL: https://github.com/apache/pekko/pull/3681
### Motivation #3668 made `PekkoForkJoinTask` delegate `hashCode()` to the wrapped `Runnable`. #3669 was opened to remove that override again, but it was abandoned, so the change is still on `main`. Reverting it before 2.0.0-M5 avoids shipping it. AI-generated reasoning on why #3668 is not a good idea (expanding on [this review comment](https://github.com/apache/pekko/pull/3668#issuecomment-6360690859)): 1. **No consumer is identified.** Nothing in Pekko calls `hashCode` on these tasks, and neither does `ForkJoinPool`, which picks queues by thread, not by task. The motivation ("wrapping the same Runnable does not preserve its hash code") states a property rather than a problem, and #3621/#3628 don't depend on it. 2. **It makes hashing worse for the main caller.** The dispatcher wraps each actor's `Mailbox` in a fresh `PekkoForkJoinTask` on every submission. Before #3668, each wrapper had its own identity hash. After it, every wrapper for a busy actor has the same hash. Anything that tracks in-flight tasks in a hash-based structure, such as monitoring or tracing agents that instrument `ForkJoinTask`, puts them all in one bucket. 3. **It makes the wrapper depend on user code.** Every call to the wrapper's `hashCode` runs the user's `Runnable.hashCode()`. A case-class runnable recomputes a structural hash each time, and a mutable one can change hash while the task is in a set. The added scaladoc ("provided that the Runnable's hash code is stable") acknowledges this. 4. **Overriding `equals` as well would make it worse.** The suggested follow-up of also overriding `equals` would make separate submissions of the same `Runnable` compare equal, even though each has its own completion and cancellation state. JDK task adapters such as `ForkJoinTask.adapt` keep identity semantics for this reason. 5. **The tests don't exercise the collision case.** `Set(cancelled, completed, pending)` is a `Set3`, which compares elements by `equals` and never hashes. ### Modification Revert #3668: remove the `hashCode` override and its scaladoc from `PekkoForkJoinTask`, and remove the `PekkoForkJoinTask` tests added to `MailboxTaskSubmissionSpec`. ### Result `PekkoForkJoinTask` is back to identity `hashCode` and `equals`, one identity per submission, as before #3668. ### Tests - Not run locally: this is a straight `git revert` back to the code before #3668, which passed CI. CI validates this PR. ### References Refs #3668, Refs #3669 (abandoned follow-up that would have removed the override). -- 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]
