stevedlawrence commented on PR #1539:
URL: https://github.com/apache/daffodil/pull/1539#issuecomment-3207407206
> The underlying mechanism of OnStack can use ThreadSafePool,
I started doing this to avoid ThreadLocal's and potential memory leaks, but
now I'm wondering if it kindof breaks the purpose of OnStack. For example, say
you have nested (or recursive) OnStack's, for a small example:
```scala
class SomeParser {
object withFoo extnds OnStack[Foo]
def parse() {
withFoo { foo1 =>
withFoo { foo2 =>
...
}
}
}
}
```
With a ThreadLocal OnStack, we know that foo1 and foo2 come from the same
stack stored in withFoo and would give good memory locality and stack-like
allocations. But if we switched OnStack to internally use a ThreadSafePool,
then OnStack would contain a Pool of Stacks. But this would mean each time you
call `withFoo` it's going to get new Stack from the pool (ThreadSafePool does
not share anything, even when nesting) and get a Foo from that stack. But this
stack withFoo returns will always have a maximum size of one. And this
definitely defeats the purpose.
So I think to achieve what OnStack does with a stack-like allocations, it
really does need to use a ThreadLocal--ThreadLocal allows nested calls to use
the same instance--ThreadSafePool does not allow sharing when nested.
So I'm not sure what the right solution is. We could switch all the OnStack
uses to use ThreadSafePool (ignoring the allocation differences, the only real
differences is the ability to have a reset() function, which we could add). We
lose the memory locality and maybe get some extra thread contention, but it
would be thread safe and not have memory leaks.
Or we just accept that OnStack can lead to memory leaks. I think the related
memory leaks would not be as bad as the ones we saw with the TDML runner and
validation since the OnStacks are all part of parsers, but I think there is
still the potential for leaks.
That said, part of me feels like OnStack is trying to force a stack
allocation-like paradigm on Java, which is inherently not stack-based, and so
we're now starting to notice the associated quirks with ThreadLocals. And maybe
moving away from OnStack is the right approach?
--
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]
