You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Reading the code for #exec_recursive and #exec_recursive_cline in #15361 I'm wondering about a couple of things regarding this recursion detection mechanism. They're unrelated to the current refactor, so I'm starting a separate discussion.
What happens when the block raises? This must be expected because the mechanism is used for writing to an IO. Apparently the entries will never get cleared. We should probable handle exceptions explicitly via ensure.
Should we release the hash when it's been emptied? We don't need to keep occupying the memory if no recursive operation is ever used in that same fiber. On the other hand, if there is another recursive operation we could reuse the previous allocation. This could be significant if there's a lot of use. However, AFAIK hash memory never shrinks, so this could become a memory leak.
Finally, Hash might be an easy to use tool for this, but I would imagine it's also quite heavy with regard to heap allocations. In typical use cases there probably won't be that many elements in the hash anyways. Its size is defined by the nesting depth of container objects. I'd bet in the majority of cases this won't exceed 16 items which is the size up to which Hash does a linear scan anyway. So a more lightweight data structure, such as stack-allocated linked list, could perform similarly.
Performance considerations might not be too critical for this use case, but it's still worth to think about.
Potential memory leaks are always a relevant concern.
The text was updated successfully, but these errors were encountered:
Keeping a linked list of nodes allocated on the stack while we're dealing with recursion, means we won't have full benefit of the contiguous memory, but should still benefit from locality since the nodes shouldn't be too far apart on the stack. Nice 👍
Additional perks: no allocation, no memory leak, automatic cleanup 👍
Reading the code for
#exec_recursive
and#exec_recursive_cline
in #15361 I'm wondering about a couple of things regarding this recursion detection mechanism. They're unrelated to the current refactor, so I'm starting a separate discussion.Hash
might be an easy to use tool for this, but I would imagine it's also quite heavy with regard to heap allocations. In typical use cases there probably won't be that many elements in the hash anyways. Its size is defined by the nesting depth of container objects. I'd bet in the majority of cases this won't exceed 16 items which is the size up to which Hash does a linear scan anyway. So a more lightweight data structure, such as stack-allocated linked list, could perform similarly.Performance considerations might not be too critical for this use case, but it's still worth to think about.
Potential memory leaks are always a relevant concern.
The text was updated successfully, but these errors were encountered: