Skip to content

Conversation

@SirOlaf
Copy link
Contributor

@SirOlaf SirOlaf commented Jul 22, 2024

Ref: #23361

Fixes the first example in referenced issue. The second example requires that pages can be inherited as creating a string allocates which then causes a cell to be moved into a foreign thread.

next and prev fields of SmallChunk are kept around for now as I suspect they will help for the more complicated case.

@SirOlaf SirOlaf changed the title WIP: Clean up dead threads Allocator: Handle basic dead threads with unshared memory Oct 17, 2024
@SirOlaf SirOlaf marked this pull request as ready for review October 17, 2024 11:48
@SirOlaf
Copy link
Contributor Author

SirOlaf commented Oct 17, 2024

I am hoping to come back to this with a more universal solution that allows allocators to adopt pages, but for most scenarios this one is perfectly usable.

Will review what I did here again first though

@SirOlaf SirOlaf marked this pull request as draft October 17, 2024 12:15
@SirOlaf SirOlaf marked this pull request as ready for review October 17, 2024 14:16
@SirOlaf SirOlaf marked this pull request as draft October 17, 2024 15:50
@SirOlaf
Copy link
Contributor Author

SirOlaf commented Oct 17, 2024

Gonna need a deeper look after all, there’s some code I really want to make work

@Araq
Copy link
Member

Araq commented Nov 5, 2024

Can you undraft this? Every little progress on this front is greatly appreciated.

@SirOlaf SirOlaf marked this pull request as ready for review November 5, 2024 13:49
@SirOlaf
Copy link
Contributor Author

SirOlaf commented Nov 5, 2024

Sure, just really threw me off when I discovered that my use case still doesn't work (as it also messes with it by reallocating a sequence or string, I guess that shouldn't work but still got me)

# Because removals from `a.freeSmallChunks[s]` only happen in the other alloc branch and during dealloc,
# we must not add it to the list if it cannot be used the next time a pointer of `size` bytes is needed.
listAdd(a.freeSmallChunks[s], c)
a.freeSmallChunks[s] = c
Copy link
Member

Choose a reason for hiding this comment

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

This makes me nervous. What is the justification for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is always only a single active chunk which receives cells when they get freed, so there is no reason for chunks to keep acting as a list in the way they did before.

Copy link
Contributor Author

@SirOlaf SirOlaf Nov 5, 2024

Choose a reason for hiding this comment

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

I was thinking of using the list behavior for the remaining threading issues to easily find chunks that must stay alive, but not sure if I can figure out how to do that yet.
Either way that was the original reason for making the change after realizing it's technically unused right now

@github-actions
Copy link
Contributor

github-actions bot commented Nov 8, 2025

This pull request is stale because it has been open for 1 year with no activity. Contribute more commits on the pull request and rebase it on the latest devel, or it will be closed in 30 days. Thank you for your contributions.

@github-actions github-actions bot added the stale Staled PR/issues; remove the label after fixing them label Nov 8, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2025

This pull request has been marked as stale and closed due to inactivity after 395 days.

@github-actions github-actions bot closed this Dec 9, 2025
@Araq Araq reopened this Dec 9, 2025
@github-actions github-actions bot removed the stale Staled PR/issues; remove the label after fixing them label Dec 11, 2025
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