Matheus Alcantara <mths.dev@pm.me> writes: > On Friday, July 29th, 2022 at 19:53, Tom Lane tgl@sss.pgh.pa.us wrote: >> I wonder if we can combine ideas from the two patches to get a >> better tradeoff of code coverage vs. runtime.
> I was checking the Pavel patch and notice that he was using the fillfactor > parameter when creating the gist index. I changed my previous patch to include > this parameter and the code coverage of gistbuild.c and gistbuildbuffers.c was > improved to 97.7% and 92.8% respectively.
Nice!
I poked at this some more, wondering if we could combine the two new index builds into one test, and eventually realized something I should probably have figured out before: if you make effective_cache_size too small, it refuses to use buffering build at all, and you get here:
if (levelStep <= 0) { elog(DEBUG1, "failed to switch to buffered GiST build"); buildstate->buildMode = GIST_BUFFERING_DISABLED; return; }
In fact, at least on my machine, the first test case hits this and thus effectively adds no coverage at all :-(. If I remove that and just add the second index build, the above-quoted bit is the *only* thing in gistbuild.c or gistbuildbuffers.c that is missed compared to using both test cases. Moreover, the runtime of the test comes down to ~240 ms, which is an increment of ~25ms instead of ~65ms. (Which shows that the non-buffering build is slower, not surprising I guess.)
I judge that covering those three lines is not worth the extra 40ms, so pushed just the second test case.
Thanks for poking at this! I'm much happier now about the state of code coverage in that area.
I'm happy, that the improvement of the tests I've forgotten about so long ago is finally committed. Thank you!