libgap API: How much (if any) of GASMAN to expose?

[embray]

Currently none of the garbage collection related functions are (intentionally) exposed in libgap-api.h. At the very least least the markBagsCallback that currently gets passed to GAP_Initialize needs to be able to call MarkBag on objects being tracked by the user.

If you didn't want to expose GASMAN internals at all (e.g. maybe for working with GAP built to use a different GC?) an alternative might be to change the markBagsCallback to simply return a pointer to an array of Bags (or some more opaque object pointer type) that the user wishes to be marked as active by the GC. So instead of the current implementation of

    /* 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. */
    if (ExtraMarkFuncBags) {
        (*ExtraMarkFuncBags)();
    }

something roughly more like

if (ExtraMarkFuncBags) {
    Bag* tmp = ExtraMarkFuncBags();
    for (tmp; tmp != NULL; tmp++) {
        MarkBag(*tmp);
    }
}

just a thought, in case you would prefer to keep GC-related details more private.

[fingolfin]

I have no problem with exposing MarkBag as GAP_MarkBag, it is supported by all three GC implementations, too, and if we ever add more, they'll also have it. BTW, you can also see that we (well, I at least) consider this acceptable by looking at https://hackmd.io/s/SJzZVOLem . So if you want to submit a PR adding it, we'll merge it.

[embray]

Okay, fine by me! It would be good to have CollectBags as well, which I also see in your list of "functions which are probably OK for the API".

[fingolfin]

Yeah, CollectBags is fine, too. I would recommend only exposing the full argument, though, so e.g. void GAP_ CollectBags(int full). Also, feel free to come up with a name where it is "easier to guess what it means"; perhaps GAP_CollectGarbage? I find it hard to judge this, as of course I am intimately familiar with the internal GAP "API", and so for me, the closer the new name is to the internal name, the "easier"; but people who just want to use the API might appreciate some fresh air.

[fingolfin]

BTW, of course we are aware that many more APIs are needed (indeed, for the various GAP-to-X interfaces I've been working on, I'd need more). But instead of blindly adding random APIs based on guesses whether somebody might or might not need them, we prefer to add them based on request resp. actual use, to make sure the APIs we add are usable and useful in practice, and not just in theory.

Among the things I expect people will want are:

• ObjInt_Int (and perhaps some of its siblings), instead of the unsafe INTOBJ_INT (which also is not part of libgap-api)
• perhaps also Int_ObjInt and some of its variants
• functions to create and access records
• more IS_FOO functions to replace tests for tnum values
• possibly still wrapper for TNUM_OBJ and TNAM_OBJ, at least for debugging purposes
• a variant of GAP_MakeString which takes a byte buffer pointer and a size (say GAP_MakeStringWithSize or GAP_MakeStringWithLength), to be more efficient (and to support strings with null bytes, but in my experience those rarely matter)
• ... that covers most of the list in the HackMD, I think, with the following exceptions
  • SHALLOW_COPY_OBJ: for this, I think it would be better to call the GAP function, as that may implement additional copy method; but if you badly need it, we could of course add a GAP_ShallowCopy function to the API which calls the GAP operation ShallowCopy for you)
  • CopyObj -> I'd recommend to call Immutable or StructuralCopy instead... but I guess we could also expose it, with a well-written comment (BTW, I noticed that the sage code for cpdef GapElement deepcopy claims to be "much faster" than calling GAP's StructuralCopy, which is ... surprising ... given that both call CopyObj)
  • ClearError -> we can expose that, but on the long run, I'd rather not, but instead get the error handling story "right". But that's not something I expect to be quickly done, so I guess we can add it to the API, with the understanding that it might go or change to a no-op in a distant future revision of the API
  • ViewObjHandler: just call the GAP operation ViewObj, for which this is the handler function.
  • ... anything else I missed?

Finally, as we indicated before, anything that is added now can probably be backported to the 4.10 series, so it would appear in 4.10.1 or 4.10.2 etc.

[embray]

BTW, of course we are aware that many more APIs are needed (indeed, for the various GAP-to-X interfaces I've been working on, I'd need more). But instead of blindly adding random APIs based on guesses whether somebody might or might not need them, we prefer to add them based on request resp. actual use, to make sure the APIs we add are usable and useful in practice, and not just in theory.

Yes, definitely. A "YAGNI" approach would be good here (which is why I was hesitant about requesting anything related to garbage collection at all).

ClearError -> we can expose that, but on the long run, I'd rather not, but instead get the error handling story "right". But that's not something I expect to be quickly done, so I guess we can add it to the API, with the understanding that it might go or change to a no-op in a distant future revision of the API

It might still prove useful even in a revised error-handling situation. Consumers of the API will want to check whether an error occurred, and possibly manually call some "ClearError" to indicate that the error has been handled somehow, and execution of the GAP kernel may proceed normally. Otherwise, one might call further GAP functions with an already set error condition, which might cause undefined or undesired behavior. Also, per my comments in #2487 I could try to work on the error-handling situation, as I have a pretty good handle on the situation now, I think...

[fingolfin]

With PR #4215 merged, can this be closed? If not, what do you think is missing from the GASMAN / GC interface?

[embray]

Thanks, I missed that this was merged.