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

Further cleanup GC code for map/list/set #1086

Merged
merged 6 commits into from
Jun 5, 2024
Merged

Further cleanup GC code for map/list/set #1086

merged 6 commits into from
Jun 5, 2024

Conversation

dwightguth
Copy link
Collaborator

This is a further cleanup of the code used to deal with map/list/set objects that are returned by functions and not immediately placed into a term. Previously, these objects lived on the alwaysgcspace and were promoted to the youngspace by placing them as a child of a special placeholder symbol if they survived to a collection cycle. However, this had the undesirable trait that it breaks the invariant originally imagined by the alwaysgcspace that it should not contain any allocations that might survive past the beginning of the next rewrite step.

Because of this, we were unable to separate the alwaysgcspace clear operation from the garbage collection cycle, something that is required for the sequence of GC refactorings I am in the progress of. To fix this, we simply change the way we allocate such map/list/set terms such that we allocate them directly to the youngspace as special placeholder symbols, rather than going through the intermediary of the alwaysgcspace. This resets the expectations that ultimately lead to the alwaysgcspace having the property that allocations might survive past the end of the current rewrite step, which allows us to make a further cleanup in the next PR which automatically resets and clears the space after every rewrite step.

@rv-jenkins rv-jenkins changed the base branch from master to develop June 4, 2024 17:59
@dwightguth dwightguth marked this pull request as ready for review June 4, 2024 18:48
@dwightguth dwightguth requested review from Baltoli and theo25 June 4, 2024 18:48
Copy link
Contributor

@Baltoli Baltoli left a comment

Choose a reason for hiding this comment

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

One question, but this looks like a nice cleanup!

@@ -151,4 +151,61 @@ __attribute__((always_inline)) void *kore_alloc_floating_old(size_t requested) {
init_with_len(result, sizeof(floating_hdr) - sizeof(blockheader));
return &result->f;
}

void *kore_alloc_map(size_t requested) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could these be sensibly factored out into a common function? As it is we have three identical bits of code, except for the tag that they are parametrised on. I might be missing something that will change in the next set of PRs, though

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe it should be possible with an inline template function. I'll take a stab at it.

@dwightguth dwightguth requested a review from Baltoli June 5, 2024 17:39
@dwightguth dwightguth merged commit 7c3ed68 into develop Jun 5, 2024
10 checks passed
@dwightguth dwightguth deleted the cleanup_gc_2 branch June 5, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants