Re: Missing CFI in hlCover()?

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
>> Hm. I'd vote for a CFI within the recursion in TS_execute(), if there's
>> not one there yet. Maybe hlFirstIndex needs one too --- if there are
>> a lot of words in the query, maybe that could be slow? Did you pin the
>> blame down any more precisely than hlCover?

> I've definitely seen hlFirstIndex take a few seconds to run (while
> running this under gdb and stepping through), so that could be a good
> choice to place one (perhaps even additionally to this...). I have to
> admit to wondering if we shouldn't consider having one in
> check_stack_depth() to try and reduce the risk of us forgetting to have
> one in sensible places, though I've not really looked at all the callers
> and that might not be reasonable in some cases (though I wonder if maybe
> we consider having a 'default' version that has a CFI, and an alternate
> that doesn't...).

Adding it to check_stack_depth doesn't really seem like a reasonable
proposal to me; aside from failing to separate concerns, running a
long time is quite distinct from taking a lot of stack.

The reason I'm eyeing TS_execute is that it involves callbacks to
functions that might be pretty complex in themselves, eg during index
scans. So that would guard a lot of territory besides hlCover. But
hlFirstIndex could use a CFI too, if you've seen it take that long.
(I wonder if we need to try to make it faster. I'd supposed that the
loop was cheap enough to be a non-problem, but with large enough
documents maybe not? It seems like converting to a hash table could
be worthwhile for a large doc.)

regards, tom lane