inference: represent callers_in_cycle with view slices of a stack instead of separate lists #55364
Conversation
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. |
vtjnash
force-pushed
the
jn/callers-in-cycle-stack
branch
2 times, most recently
from
a9463b1
to
94310a3
Compare
vtjnash
force-pushed
the
jn/callers-in-cycle-stack
branch
from
94310a3
to
a56c482
Compare
vtjnash
force-pushed
the
jn/callers-in-cycle-stack
branch
2 times, most recently
from
ec605ec
to
c4b0b77
Compare
Inspired by Tarjan's SCC, this changes the recursion representation to use a single list instead of a linked-list + merged array of cycles. Fix a doopt boolean that got apparently incorrectly flipped as long ago as 5f10eb9, and then further mis-propagated by later PRs until we were attempting to return objects that were also being put into the cache, which is unsafe if the user decides to mutate them later.
aviatesk
force-pushed
the
jn/callers-in-cycle-stack
branch
from
c4b0b77
to
215799a
Compare
|
@nanosoldier |
| frame_parent(sv::IRInterpretationState) = sv.parentid == 0 ? nothing : (sv.callstack::Vector{AbsIntState})[sv.parentid] | ||
|
|
||
| # add the orphan child to the parent and the parent to the child | ||
| function assign_parentchild!(child::InferenceState, parent::AbsIntState) |
There was a problem hiding this comment.
Would it be safer to create a new constructor InferenceState(..., parent::AbsIntState) that internally calls assign_parentchild! and use it in IPO inference situations, rather than using assign_parentchild! individually in each case?
There was a problem hiding this comment.
I think eventually we may like to move this state into the AbstractInterpreter, since it doesn't properly belong to this InferenceState frame, but rather to the process as a whole. But some of it depends on whether and how we want to parallelize this process. For now, this was a more minimal change to eventually permit pause-able inference on a single thread.
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. |
Co-authored-by: Shuhei Kadowaki <[email protected]>
…tead of separate lists (#55364) Inspired by Tarjan's SCC, this changes the recursion representation to use a single list instead of a linked-list + merged array of cycles.
Inspired by Tarjan's SCC, this changes the recursion representation to use a single list instead of a linked-list + merged array of cycles. I have wanted to do this refactoring (from linked list to array) for a while.
@nanosoldier
runbenchmarks(!"scalar", vs=":master")