Add Handler for Copying Immutable Plists and Strings

[markuspf]

This does the same for Plists and String Objects as is done for Blists in #1514, and adds GAP_ASSERTs

[codecov]

Codecov Report

Merging #1520 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1520      +/-   ##
==========================================
- Coverage   64.19%   64.18%   -0.01%     
==========================================
  Files         992      992              
  Lines      322094   322631     +537     
  Branches    13072    13243     +171     
==========================================
+ Hits       206755   207092     +337     
- Misses     112521   112688     +167     
- Partials     2818     2851      +33

Impacted Files	Coverage Δ
src/stringobj.c	76.72% <100%> (+0.05%)	⬆️
src/plist.c	88.23% <100%> (ø)	⬆️
src/objects.h	77.08% <0%> (-2.92%)	⬇️
src/hpc/thread.c	46.64% <0%> (-0.2%)	⬇️
src/hpc/threadapi.c	31.4% <0%> (-0.1%)	⬇️
lib/cmdledit.g	6.49% <0%> (-0.05%)	⬇️
src/stats.c	72.66% <0%> (+0.13%)	⬆️
src/funcs.c	69.94% <0%> (+0.14%)	⬆️
src/gap.c	57.6% <0%> (+0.29%)	⬆️
src/hpc/traverse.c	79.77% <0%> (+0.38%)	⬆️
... and 3 more

[markuspf]

Now also fixes a bug in the installation of string CopyObjFunc handlers.

[stevelinton]

CopyBlistImm, CopyPlistImm an CopyStringImm are all actually the same function. Not sure if it is worth combining them.

[fingolfin]

What exactly does this gain us? We seem to replace a single function by two, which opens up the possibility for ambiguity, so we end up adding assertions to detect just the ambiguity we just introduced.
Of course with assertions turned off, this might shave off a few CPU cycles for each immutable object we copy. But is that really worth it?

[stevelinton]

@fingolfin I think you either buy into the dispatch tables or you don't. If you do then it is absolutely natural that immutable objects, which have a different TNUM are handled by a different function. The asserts are basically just to check against someone in the kernel calling the handler directly rather than via the dispatch.

[fingolfin]

@stevelinton I generally do "buy" into the dispatch tables, but I have my doubts for this particular case.

Indeed, I was thinking about the whole "IMMUTABLE is a bit in the TNUM" business. It might actually simplify things to get rid of that, and turn IMMUTABLE into an object flag using TEST_OBJ_FLAG / SET_OBJ_FLAG / CLEAR_OBJ_FLAG (I'd rename it to e.g. IMMUTABLE_FLAG to help detect old code). Then, enhance IS_MUTABLE_OBJ with a hotpath: replace

#define IS_MUTABLE_OBJ(obj) \
                        ((*IsMutableObjFuncs[ TNUM_OBJ(obj) ])( obj ))

by

static inline int IS_MUTABLE_OBJ(Obj obj) {
  UInt tnum = TNUM_OBJ(obj);
  if (tnum <= LAST_IMM_MUT_TNUM)
    return TEST_OBJ_FLAG(obj, IMMUTABLE_FLAG);
  return ((*IsMutableObjFuncs[ tnum ])( obj ));
}

[stevelinton]

@fingolfin That makes sense. I hadn't come across OBJ_FLAGS before. Way back when, object headers were 2 32 bit words and there wasn't room for anything else. Now I guess there are a few spare bits even in 64 bit, as long as we think it's OK to limit object sizes to 256TB or so (should be OK for a few more years).

We should probably use them for copying as well.

[fingolfin]

If we hit the dreaded 48 bit / 256 TB barrier one day, we can simply adjust BagHeader to reserver more bits for the object size (right now we still have a few unused bits). We might even consider increasing the header size by a full word, given that we'll have plenty of RAM to spare.

So I am not concerned ;-)

And yes, COPYING is also there, but it's a bit of a different story. There, we actually modify the content of the object, so any object which has the COPYING marker tends to be invalid for all methods operating on that object. So no such methods must be invoked on them -- and right now, this does indeed lead to errors, thanks to our dispatching system. If we got rid of that, we'd have to be very careful to adjust all code to check the COPYING flag. All of this is perhaps not so much a problem for GAP, but certainly for HPC-GAP.

Anyway, that's why I would kind of prefer to rewrite the COPYING code, say by using the traversal code introduced by HPC-GAP. @rbehrends made some ominous remarks on how one could be much faster by somehow using an array plus a "COPIED" bit, but I have no idea what he had in mind, and he has not responded to my request for an explanation.

[stevelinton]

All sounds sensible to me.

[fingolfin]

Closing this in favor of PR #1563, #1571 etc.