AaronBallman wrote:
> > I don't have strong opinions about the approach taken here, but it seems
> > like we're punishing the simple case of a single level of templates in
> > order to better diagnose more pathological cases.
>
> Just a short brain-storming:
>
> As I understand, the current StackHandler.runWithSufficientStackSpace() does
> not differentiate the recursion level at the moment : the same checks are
> done for 1 as well as for 1000 levels of recursion.
Correct.
> What if the StackHandler would track the recursion level and do something
> like:
>
> ```
> if (recursion_level < threshold_level) [[ likely ]] {
> F(); // just recurse normaly
> } else {
> // the current logic
> // if (isStackNearlyExhausted()) [[ unlikely ]] {...} else {...}
> }
> ```
`>
> What we want to achieve with this:
>
> * up to some small (threshold) number of recursions just recurse normally
> ( by assuming we will never exceed stack limit on this recursion level)
>
> * for deeper recursions start doing more checks.
>
> * different functions may have different thresholds
>
>
> Some rough idea: https://godbolt.org/z/ovvMY46zT
>
> Of course, measurements would have to confirm benefits of such an approach,
> if any at all.
>
> But for the case it's positive, it would cover all scenarios: cheap for small
> recursion depth and robust otherwise.
>
> Or formulated differently: Is there a way to make this robust but cheap for
> the normal usage (zero-cost principle) ?
>
> Just an idea.
That's an interesting idea and might be a workable general solution. I think
eliminating the recursion via tail calls is likely still a better approach when
we can achieve it though. We have other problems beyond just stack resource
limits; for example, the more memory pressure we apply the fewer recursive
templates we can instantiate due to running into out of memory scenarios. By
reducing stack usage, we also help with that kind of thing. However, I think
there's even more low-hanging fruit for that situation like paying attention to
stack frame sizes (I've seen some *gigantic* stack frames when I was last
poking around with this issue). But for the times when we just can't get tail
calls to work, having a smarter `runWithSufficientStackSpace()` is attractive
enough to be worth exploring.
Now, whether we need to do that as part of this PR or not is a whole different
question. Given the performance behavior @zyn0217 posted about, maybe it's fine
to move forward with this as-is. I think Clang two-stage uses enough templates
to be a reasonable proxy for the scenario I was worried we'd be slowing down.
https://github.com/llvm/llvm-project/pull/224393
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
