pjfanning commented on PR #3668:
URL: https://github.com/apache/pekko/pull/3668#issuecomment-6360690859
I'm withdrawing my earlier approval: on a closer look I can't see why this
change is needed. Delegating `hashCode` while `equals` stays identity-based is
legal, but:
1. **No consumer is identified.** Nothing in Pekko calls `hashCode` on these
tasks, and `ForkJoinPool` doesn't either, since queue selection is by thread,
not 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. Today
each wrapper gets its own identity hash. With this change, every wrapper for a
busy actor has the same hash. Anything that keeps in-flight tasks in a hash
map, such as monitoring agents that instrument `ForkJoinTask`, would put them
all in one bucket.
3. **It makes the wrapper depend on user code.** The wrapper's `hashCode`
now runs the user's `Runnable.hashCode()` on every call. 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 scaladoc's "provided that the Runnable's hash code is
stable" acknowledges this.
4. **Test nit, as you noted.** `Set(cancelled, completed, pending)` is a
`Set3`, which never hashes, so it doesn't exercise the collision case.
If there's a concrete consumer that needs a stable hash, could you add it to
the motivation so we can weigh it against point 2? Otherwise I'd suggest
closing this.
--
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]
