Add ExtraMarkFuncBags

[markuspf]

This allows users of gasman to use a custom bag marking function
that is called during garbage collection.

The primary reason for adding this is so that one can keep bags
alive when using GAP as a dynamic library.

There is currently no HPC-GAP support, and no tests, which is of course bad.

[markuspf]

The previous version of this patch had getter/setter functions for the ExtraMarkFuncBags variable; This was mainly there because libgap had it.

A setter/getter combination might be a good idea for the library-api (though I can't think of any reasons right now).

[stevelinton]

I thought about this problem once and envisaged a slightly different solution -- in my model whenever you handed a gap Obj outside the GAP library you add it to a global list (or some better datastructure). You also provide an API for the outer programme to release objects when it has finished with them (perhaps as a hook from its own memory management.

This seems to be kind of the opposite, the outer programme, via the ExtraMarkFunc... has to say what bags it wants kept alive at any given moment.

Do you have a reason for prefering this approach?

[markuspf]

Actually I implemented the approach you suggest (partially) in Kaiserslautern earlier this year.

I ported forward parts of #1205 which in turn was just what we got from the Sage project without thinking much, so I will look into this issue again. Thanks for reminding me that there might be a better approach.

[fingolfin]

I have a hard time imagining what this would be used for.
Can't you just use a single plist to store all bags you want to keep alive, and then tell GASMAN about this list using InitGlobalBag?

Perhaps an example of how this would actually be used would clarify this?

[fingolfin]

this is not used in GAP, and thus somebody who removes dead code might do so.

So perhaps make it explicit that this is there for the benefit of code linking against GAP?

[fingolfin]

This point really is my only objection to this patch: I still think this comment is potentially misleading, or at least not as helpful as it could be. Perhaps this:

/* allow installing a custom marking function. This is used for integrating GAP (possibly linked as a shared library) with other code bases which use their own form of garbage collection. For example, with Python (for SageMath) or Julia. */

This is just meant to convey the gist, I am sure it can be improved a lot.

[markuspf]

As I said above, this is a port of what I got from @vbraun, and I forgot that I had implemented a retain-list in Kaiserslautern. I seem to remember that we need reference counting instead of a retain list (which one can do by using an ObjMap).

This can then be part of a set of convenience functions in the GAP library API (with documentation) which is a whole lot cleaner than this approach.

[rbehrends]

A custom routine for supplying additional GC roots would be a good idea, IMO. Applications range from marking coroutine stacks to code where maintaining a separate list of roots is expensive, but where it's easy to find all of them at any given time (we actually have a very similar problem with Singular/Julia interaction). The tricky part with the "list of roots" approach is deletion from the list. You either need a (slow) dictionary or need to store the position in the list alongside the object reference, leading effectively to fat pointers.

So, I'd be in favor of having a more general and more efficient approach.

That said, I have a couple of technical comments about the pull request.

• I think that such a mechanism should not be limited to a single client.
• This is not really a TNum-related mechanism, so I'd name it differently.

[markuspf]

When you say "should not be limited to a single client", you mean you want a list of marking functions? To use @fingolfin's words: I have a bit of a hard time imagining a use-case for this, so could you elaborate on that?

The reason I called it TNum... is that other functions in the vicinity are also called TNum...; happy to change that since it seems nonsensical to me, too.

[vbraun]

For the record, I initially tried this approach of keeping objects alive by having them referenced by some GAP global. But then you need to communicate deletion back into gap. Which could happen at any time, so you need to be careful about when to mark the stack bottom. I found it easier to keep the garbage collection in GAP and Python to be as far separated as possible.

[fingolfin]

@vbraun it's not quite clear to me what the concrete conclusion of your statement now is... are you supporting the PR as-is? Or are you suggesting changes, and which?

[vbraun]

Sorry, let me try again: A hook to simply mark bags alive (ExtraMarkFunc) is in my experience easier to use than an interface where I have to call GAP whenever I want an externally-referenced object to be alive/dead. Source: I tried both in Sage.

TL;DR: I support this PR.

[fingolfin]

Please add a comment as suggested

[markuspf]

I added the comment.

[fingolfin]

Hum... but nothing ever sets this to a non-zero value; and no header mentions this.

So code wanting to set it would have to include extern TNumExtraMarkFuncBags ExtraMarkFuncBags; ?