Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add GAP_MarkBag, GAP_CollectBags, a minimal interface to the garbage collector #4215

Merged
merged 2 commits into from
Jan 27, 2021

Conversation

embray
Copy link
Contributor

@embray embray commented Jan 18, 2021

Add libgap interfaces to the garbage collector. As discussed in #3030
these are probably the minimal interfaces that need to be exposed, and
are compatible with all current GC implementations.

As suggested at #3030 (comment)
GAP_CollectBags only takes a full argument, but not size.

Copy-editing may be needed on the documentation the header.

…ystem#3030

these are probably the minimal interfaces that need to be exposed, and
are compatible with all current GC implementations.

As suggested at gap-system#3030 (comment)
GAP_CollectBags only takes a <full> argument, but not <size>
Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me but it'd be nice if @ChrisJefferson also had a look

src/libgap-api.c Outdated Show resolved Hide resolved
src/libgap-api.h Outdated Show resolved Hide resolved
src/libgap-api.h Outdated Show resolved Hide resolved
@fingolfin fingolfin added topic: kernel topic: libgap things related to libgap labels Jan 19, 2021
Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the liberty of applying some tweaks, to make the tests pass and to address some minor nitpicks of mine. Hope that was OK, of course we can still revise!

Still would like a second opinion by @ChrisJefferson . But in general this seems to me to be OK to merge

@ChrisJefferson
Copy link
Contributor

This is fine to merge with me.

I point out the existence of InitGlobalBag, it might be places where you are using MarkBag you could be using InitGlobalBag if you only set some things up once. I wouldn't bother adding a wrapper for it until someone wants it.

@fingolfin fingolfin merged commit 653dfd1 into gap-system:master Jan 27, 2021
@ThomasBreuer ThomasBreuer self-assigned this Feb 16, 2021
@embray embray deleted the issue-3030 branch February 16, 2021 14:22
@ThomasBreuer ThomasBreuer changed the title libgap API: expose minimal interface to garbage collector libgap API: Expose a minimal interface to the garbage collector, via GAP_MarkBag, GAP_CollectBags Feb 16, 2021
@ThomasBreuer ThomasBreuer removed their assignment Feb 16, 2021
@ThomasBreuer ThomasBreuer added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Feb 16, 2021
@fingolfin fingolfin changed the title libgap API: Expose a minimal interface to the garbage collector, via GAP_MarkBag, GAP_CollectBags Add GAP_MarkBag, GAP_CollectBags, a minimal interface to the garbage collector Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: added PRs introducing changes that have since been mentioned in the release notes topic: kernel topic: libgap things related to libgap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants