SegSetGrey is dominating the critical path

[rptb1]

SegSetGrey is occupying a lot of CPU time on the critical path in sampling profiles.

keybase://chat/ravenbrook#mps/2232 by @rptb1 of 2022-12-14 16:59 UTC says:

Hmm... gprof says

  %   cumulative   self              self     total           
  4.96      3.09     0.24 15165864     0.00     0.00  SegSetGrey

which is a bit surprising. Changing the AVERs in SegSetGrey to CRITICAL. Before:

rb@plover:~/git/mps/code$ time lii6gc/hot/gcbench -x 6415986 amc
amc: 7.37883

real	0m7,398s
user	0m7,196s
sys	0m0,201s

after:

rb@plover:~/git/mps/code$ time lii6gc/hot/gcbench -x 6415986 amc
amc: 5.70199

real	0m5,729s
user	0m5,353s
sys	0m0,376s

Do I understand this properly? Nope!

[rptb1]

This performance regression was introduced gradually by several changes. SegSetGrey was recognized as being on the critical path in the past but extra assertions were introduced with segment classes.

Compare

mps/code/seg.c

Lines 1578 to 1585 in 081d79e

	static void gcSegSetGrey(Seg seg, TraceSet grey)
	{
	AVERT_CRITICAL(Seg, seg); /* .seg.method.check */
	AVERT_CRITICAL(TraceSet, grey); /* .seg.method.check */
	AVER_CRITICAL(seg->rankSet != RankSetEMPTY);
	gcSegSetGreyInternal(seg, seg->grey, grey); /* do the work */
	}

with

mps/code/seg.c

Lines 237 to 254 in 081d79e

	/* SegSetGrey -- change the greyness of a segment
	*
	* Sets the segment greyness to the trace set grey.
	*/
	void SegSetGrey(Seg seg, TraceSet grey)
	{
	AVERT(Seg, seg);
	AVERT(TraceSet, grey);
	AVER(grey == TraceSetEMPTY || SegRankSet(seg) != RankSetEMPTY);
	/* Don't dispatch to the class method if there's no actual change in
	greyness, or if the segment doesn't contain any references. */
	if (grey != SegGrey(seg) && SegRankSet(seg) != RankSetEMPTY)
	Method(Seg, seg, setGrey)(seg, grey);
	EVENT3(SegSetGrey, PoolArena(SegPool(seg)), seg, grey);
	}

We could track down where this happened if we really wanted.

[rptb1]

Some suggestions for prevention:

1. Extra review rules for the critical path. I think this has been suggested by @thejayps in another context.
2. Better profiling support. We can try to make profiling much easier for developers so that they can at least eyeball it before committing changes or submitting pull requests. gp.gmk has gone rusty for a start (see keybase://chat/ravenbrook#mps/2234 )
3. Automated performance regression tests.

[rptb1]

2. Better profiling support.

See also #138 (comment)

[rptb1]

Consider shorting out the segment class dispatch (calling gcSegSetGrey directly) here (and similar)

mps/code/poolamc.c

Line 1651 in 9bc668f

SegSetGrey(toSeg, TraceSetUnion(SegGrey(toSeg), grey));

We know that the destination toSeg is a GC segment.