You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "He-Pin (via GitHub)" <gi...@apache.org> on 2026/05/31 20:22:15 UTC
[PR] fix: release references to completed stage logics in GraphInterpreter [pekko]
He-Pin opened a new pull request, #3030:
URL: https://github.com/apache/pekko/pull/3030
### Motivation
When multiple stages are fused, a single alive stage keeps the GraphInterpreter alive and with it references to all already completed GraphStage logics, which may keep a significant amount of memory.
This issue was originally reported in akka/akka-core#23439.
### Modification
- Modified GraphInterpreter.finish() to release references to all stage logics after finalization
- Modified GraphInterpreter.toSnapshot() to handle null logics gracefully
- Added null checks in finish() to avoid NPE when logic is already null
### Result
GraphInterpreter no longer keeps references to completed stage logics, allowing them to be garbage collected even if the interpreter is still alive due to other fused stages.
### Tests
- Added directional test to verify logics are released after finish()
- Added test to verify logics are released when stages complete early
### References
- Fixes #1234 (GraphInterpreter should not keep references to shut-down logics)
- Related to akka/akka-core#23439
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: release references to completed stage logics in GraphInterpreter [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #3030:
URL: https://github.com/apache/pekko/pull/3030#discussion_r3539306279
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/GraphInterpreter.scala:
##########
@@ -342,10 +342,13 @@ import pekko.stream.stage._
var i = 0
while (i < logics.length) {
val logic = logics(i)
- if (!isStageCompleted(logic) && !isStageFinalized(logic)) {
+ if ((logic ne null) && !isStageCompleted(logic) && !isStageFinalized(logic)) {
markStageFinalized(logic)
finalizeStage(logic)
}
+ // Release reference to the stage logic so it can be garbage collected
+ // even if the GraphInterpreter is still alive due to other references
+ logics(i) = null
Review Comment:
@He-Pin can you comment on why this should be treated as invalid or what changes that you have made to fix 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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: release references to completed stage logics in GraphInterpreter [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #3030:
URL: https://github.com/apache/pekko/pull/3030#issuecomment-4780622120
oops, will rebase and fix soon. the origin problem was reported by @jrudolph
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: release references to completed stage logics in GraphInterpreter [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #3030:
URL: https://github.com/apache/pekko/pull/3030#discussion_r3557761683
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/GraphInterpreter.scala:
##########
@@ -13,6 +13,7 @@
package org.apache.pekko.stream.impl.fusing
+import java.util.BitSet
Review Comment:
Maybe we can just use a plain loop as it before?
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: release references to completed stage logics in GraphInterpreter [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #3030:
URL: https://github.com/apache/pekko/pull/3030#discussion_r3557426580
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/GraphInterpreter.scala:
##########
@@ -13,6 +13,7 @@
package org.apache.pekko.stream.impl.fusing
+import java.util.BitSet
Review Comment:
java.util.BitSet can have absolutely terrible performance
java.util.BitSet has a few well-known performance characteristics that bite people in specific scenarios:
Where it struggles
Sparse bit patterns. BitSet allocates a long[] word array sized to the highest set bit. If you set bit 10,000,000 while only having a handful of bits set overall, you still pay for ~1.25MB of backing array. Iteration (nextSetBit) then has to skip over huge runs of zero words.
Growth/resizing. Setting a bit beyond the current capacity triggers array reallocation and copy, similar to ArrayList — can be costly if you don't pre-size it via the constructor.
Not thread-safe, so concurrent access requires external synchronization, which people sometimes don't realize until it's a bottleneck.
Boxing/indirection overhead relative to raw primitive array manipulation, if you're doing very hot-loop bit twiddling — though this is usually minor compared to the sparse-layout issue.
Common alternatives with sparse layouts
RoaringBitmap — the most widely used one in the JVM ecosystem. It partitions the bit space into 2^16-sized "chunks" and picks the best internal representation per chunk (array of shorts for sparse, bitmap for dense, run-length encoding for runs). Excellent for both sparse and clustered-dense data, very fast unions/intersections/differences, and has great real-world adoption (Lucene, Spark, Druid, Elasticsearch all use it or a variant).
JavaEWAH (Compressed bitmap) — word-aligned hybrid run-length encoding, good when you have long runs of zeros/ones, common in bitmap-index use cases (analytics/OLAP).
Sparse hash-set-of-longs approaches (e.g. using a LongOpenHashSet from fastutil, or a custom compressed trie) if you genuinely just have a sparse handful of bits and don't need bitmap set-algebra.
fastutil / Eclipse Collections primitive collections — not bitmaps per se, but relevant if BitSet was being (mis)used just as a compact int-set.
Roaring64Bitmap / Roaring64NavigableMap — for when your bit indices exceed 32 bits.
Rule of thumb
Dense, bounded-range bits (e.g., flags over a known small universe) → BitSet is fine and often the fastest option due to JIT-friendly word operations.
Sparse or very large/unbounded index space, or you need fast boolean set-algebra (AND/OR/XOR/ANDNOT) at scale → RoaringBitmap is the standard choice today.
Sequential/long-run bitmaps (columnar/analytics indexes) → EWAH-style compression tends to win.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: release references to completed stage logics in GraphInterpreter [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #3030:
URL: https://github.com/apache/pekko/pull/3030#issuecomment-4902791155
Rebased onto origin/main and addressed all Copilot review feedback:
**Review comment 1** (test identity stage instances): Already fixed — same `identityStage` instance is reused for builder and both connections.
**Review comment 2** (nulling only in `finish()`): Fixed — `afterStageHasRun()` now scans ALL logics for completed stages (not just the active stage parameter) and nulls `logics(i)` + connection handlers for finalized stages. This ensures completed stage logics are released during normal stream execution, not just at interpreter shutdown.
**Review comment 3** (Connection strong references): Fixed — `finish()` now nulls all four Connection fields: `inHandler`, `outHandler`, `inOwner`, `outOwner`. `afterStageHasRun()` nulls `inHandler`/`outHandler` for finalized stages (but preserves `inOwner`/`outOwner` since they may be needed by subsequent event processing on the same connection). Debug name methods (`inOwnerName`, `outOwnerName`, `inLogicName`, `outLogicName`) have null guards. `toSnapshot` filters connections with null owners.
**Additional improvements:**
- `toSnapshot` uses `connection.inOwner.stageId` for positional indexing instead of identity-based map lookups
- Added null guard for `logic` in `finish()` to handle idempotent calls
- `GraphStageLogicSpec` "not double-terminate" test saves logic references before `execute()`
**Test results:** GraphInterpreterSpec 13/13, GraphStageLogicSpec 17/17, InterpreterSpec+Lifecycle+Supervision+FailureModes+ActorGraphInterpreter 65/65 — all passed.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: release references to completed stage logics in GraphInterpreter [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #3030:
URL: https://github.com/apache/pekko/pull/3030#issuecomment-4780535912
@He-Pin there are merge conflicts and also open comments from Copilot that need actions.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: release references to completed stage logics in GraphInterpreter [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #3030:
URL: https://github.com/apache/pekko/pull/3030#issuecomment-4932930693
@pjfanning wdyt
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: release references to completed stage logics in GraphInterpreter [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #3030:
URL: https://github.com/apache/pekko/pull/3030#discussion_r3550872822
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/GraphInterpreter.scala:
##########
@@ -342,10 +342,13 @@ import pekko.stream.stage._
var i = 0
while (i < logics.length) {
val logic = logics(i)
- if (!isStageCompleted(logic) && !isStageFinalized(logic)) {
+ if ((logic ne null) && !isStageCompleted(logic) && !isStageFinalized(logic)) {
markStageFinalized(logic)
finalizeStage(logic)
}
+ // Release reference to the stage logic so it can be garbage collected
+ // even if the GraphInterpreter is still alive due to other references
+ logics(i) = null
Review Comment:
@He-Pin ping - for me, I'd like to see a response on this before approving this PR
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: release references to completed stage logics in GraphInterpreter [pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #3030:
URL: https://github.com/apache/pekko/pull/3030#discussion_r3557845882
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/GraphInterpreter.scala:
##########
@@ -13,6 +13,7 @@
package org.apache.pekko.stream.impl.fusing
+import java.util.BitSet
Review Comment:
I'm struggling to review this PR. I much prefer when coders commit review changes separately and don't keep squashing the code. We squash it when we merge anyway. We lose all the history of the review when the the coder forces a new squashed commit. It is sometimes useful when there are lots of commits to squash some or all of the commits but generally, it is easier for reviewers if this does not happen.
So in short, I don't know what this PR looked like before the BitSet was introduced.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: release references to completed stage logics in GraphInterpreter [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #3030:
URL: https://github.com/apache/pekko/pull/3030#issuecomment-4911200212
@pjfanning I have address all the problems
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: release references to completed stage logics in GraphInterpreter [pekko]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3030:
URL: https://github.com/apache/pekko/pull/3030#discussion_r3330888788
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/GraphInterpreter.scala:
##########
@@ -342,10 +342,13 @@ import pekko.stream.stage._
var i = 0
while (i < logics.length) {
val logic = logics(i)
- if (!isStageCompleted(logic) && !isStageFinalized(logic)) {
+ if ((logic ne null) && !isStageCompleted(logic) && !isStageFinalized(logic)) {
markStageFinalized(logic)
finalizeStage(logic)
}
+ // Release reference to the stage logic so it can be garbage collected
+ // even if the GraphInterpreter is still alive due to other references
+ logics(i) = null
Review Comment:
Nulling the logic array only in `finish()` does not release stages that complete while the interpreter keeps running: normal stage completion is finalized from `afterStageHasRun`, and `finish()` is only invoked when the interpreter is being aborted/disposed. The completed stage logics will still be retained in `logics` for the lifetime of a still-active fused interpreter, which is the leak this change is intended to address.
##########
stream-tests/src/test/scala/org/apache/pekko/stream/impl/fusing/GraphInterpreterSpec.scala:
##########
@@ -315,6 +315,80 @@ class GraphInterpreterSpec extends StreamSpec with GraphInterpreterSpecKit {
interpreter.isSuspended should be(false)
}
+ "release references to completed stage logics to prevent memory leaks" in new TestSetup {
+ val source = new UpstreamProbe[Int]("source")
+ val sink = new DownstreamProbe[Int]("sink")
+
+ builder(GraphStages.identity[Int])
+ .connect(source, GraphStages.identity[Int].in)
+ .connect(GraphStages.identity[Int].out, sink)
Review Comment:
This uses three separate `GraphStages.identity[Int]` instances: the stage passed to `builder`, the inlet passed to the first `connect`, and the outlet passed to the second `connect`. `createConnections` only knows ports from the stage instance registered with the builder, so `init()` will fail to find the owners for these ports. Reuse the same identity stage instance for the builder and both connections.
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/GraphInterpreter.scala:
##########
@@ -342,10 +342,13 @@ import pekko.stream.stage._
var i = 0
while (i < logics.length) {
val logic = logics(i)
- if (!isStageCompleted(logic) && !isStageFinalized(logic)) {
+ if ((logic ne null) && !isStageCompleted(logic) && !isStageFinalized(logic)) {
markStageFinalized(logic)
finalizeStage(logic)
}
+ // Release reference to the stage logic so it can be garbage collected
+ // even if the GraphInterpreter is still alive due to other references
+ logics(i) = null
Review Comment:
Clearing `logics(i)` does not actually make the stage logic collectible because every `Connection` still keeps strong references to its `inOwner`, `outOwner`, `inHandler`, and `outHandler` (the owners are `GraphStageLogic` instances). As a result, an interpreter retained after `finish()` still retains the completed logics through `connections`, so the intended memory release is incomplete.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: release references to completed stage logics in GraphInterpreter [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #3030:
URL: https://github.com/apache/pekko/pull/3030#discussion_r3558062476
##########
stream/src/main/scala/org/apache/pekko/stream/impl/fusing/GraphInterpreter.scala:
##########
@@ -13,6 +13,7 @@
package org.apache.pekko.stream.impl.fusing
+import java.util.BitSet
Review Comment:
fixed now. maybe we can defer this to later realse.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] fix: release references to completed stage logics in GraphInterpreter [pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin merged PR #3030:
URL: https://github.com/apache/pekko/pull/3030
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org