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

Change q050's nextList from q075 to q100, to reduce the call stack depth of method PoolChunkList.add(chunk) #13622

Open
wants to merge 1 commit into
base: 4.1
Choose a base branch
from

Conversation

laosijikaichele
Copy link
Contributor

@laosijikaichele laosijikaichele commented Sep 19, 2023

Motivation:

In PoolArena, the q050's nextList should be q100.

Because when we call method PoolChunkList.add(chunk) to move a chunk, if a chunk moved from q050 to q075, it will eventually move to q100 after the call finished.

So we can make q050.nextList = q100 in PoolArena's constructor, to reduce the call stack depth of method PoolChunkList.add(chunk).

Modification:

Change q050's nextList from q075 to q100 in PoolArena's constructor.

Result:

Reduced the call stack depth of method PoolChunkList.add(chunk).

@normanmaurer
Copy link
Member

@chrisvest @franz1981 @trustin can you please have a look

@chrisvest
Copy link
Contributor

when we call method PoolChunkList.add(chunk) to move a chunk, if a chunk moved from q050 to q075, it will eventually move to q100 after the call finished.

I don't understand why this is a given.

If the call stack depth is a problem, we can make the add method iterative instead of recursive.
If the 75-percentile list serves little purpose (it's last resort in allocation order), we could perhaps remove it entirely?

@normanmaurer
Copy link
Member

when we call method PoolChunkList.add(chunk) to move a chunk, if a chunk moved from q050 to q075, it will eventually move to q100 after the call finished.

I don't understand why this is a given.

If the call stack depth is a problem, we can make the add method iterative instead of recursive. If the 75-percentile list serves little purpose (it's last resort in allocation order), we could perhaps remove it entirely?

I agree... IMHO we can just close this

@laosijikaichele
Copy link
Contributor Author

laosijikaichele commented Jan 13, 2024

@chrisvest

I don't understand why this is a given.

Because under current(4.1 branch) implementation, for the allocation order, when the chunk moving from q050 -> q075->q100, it will NEVER stop moving on q075, which means the chunk will move to q100 once it leaves q050, so the q075 is just a temporary transit and thus is not needed in allocation order.

If the call stack depth is a problem, we can make the add method iterative instead of recursive.

The gain of reducing only one stack depth is trivial, but it is still a positive trivial change, IMHO.
We can skip q075 in allocation order by simply not link it in PoolArena's constructor, without need to modify the add method, I think this is the most simple way to approach.

If the 75-percentile list serves little purpose (it's last resort in allocation order), we could perhaps remove it entirely?

We can't remove the q075 entirely, because although the q075 is not needed in allocation order, but it is still needed in de-allocation order, as the following code shows in PoolArena's constructor:

q100.prevList(q075);
q075.prevList(q050);

The following diagrams show the chunkList nodes links.

Before this PR:

截屏2024-01-13 下午6 40 35

After this PR:

截屏2024-01-13 下午6 40 43

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.

None yet

3 participants