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

mempool allocate bug? #14669

Closed
srxqds opened this issue May 28, 2019 · 2 comments
Labels

Comments

@srxqds
Copy link

@srxqds srxqds commented May 28, 2019

In the function mono_mempool_alloc0 why not reduce pool->pos with size if G_UNLIKELY (pool->pos >= pool->end) is true?

gpointer
(mono_mempool_alloc0) (MonoMemPool *pool, guint size)
{
	gpointer rval;

	// For the fast path, repeat the first few lines of mono_mempool_alloc
	size = ALIGN_SIZE (size);
	rval = pool->pos;
	pool->pos = (guint8*)rval + size;

	// If that doesn't work fall back on mono_mempool_alloc to handle new chunk allocation
	if (G_UNLIKELY (pool->pos >= pool->end)) {
		rval = mono_mempool_alloc (pool, size);
	}
#ifdef TRACE_ALLOCATIONS
	else if (pool == mono_get_corlib ()->mempool) {
		mono_backtrace (size);
	}
#endif

	memset (rval, 0, size);
	return rval;
}

It may a bug of wasting memory?
I think it should add pool->pos -= size; after if statement.

if (G_UNLIKELY (pool->pos >= pool->end)) {
                pool->pos -= size;
		rval = mono_mempool_alloc (pool, size);
	}
@jaykrell

This comment has been minimized.

Copy link
Collaborator

@jaykrell jaykrell commented May 28, 2019

It's not as bad as it seems.

If the allocation is not large, the memory is going to be lost anyway.

It is only wasted if this is a large (>= MONO_MEMPOOL_PREFER_INDIVIDUAL_ALLOCATION_SIZE) allocation but there is room for more small (< MONO_MEMPOOL_PREFER_INDIVIDUAL_ALLOCATION_SIZE) allocations.

It does seem to be what was intended though, and the non-0 case does this.

jaykrell pushed a commit to jaykrell/mono that referenced this issue May 28, 2019
when the allocation does not fit, i.e. when it is >= MONO_MEMPOOL_PREFER_INDIVIDUAL_ALLOCATION_SIZE.

"fixes" mono#14669.
@jaykrell

This comment has been minimized.

Copy link
Collaborator

@jaykrell jaykrell commented May 28, 2019

Even that might not be true. There really might be nothing lost here at all.
But the code could be clearer.

jaykrell pushed a commit to jaykrell/mono that referenced this issue May 28, 2019
when the allocation does not fit, i.e. when it is >= MONO_MEMPOOL_PREFER_INDIVIDUAL_ALLOCATION_SIZE.

Or at least make the code clearer and smaller.

"fixes" mono#14669.
@srxqds srxqds closed this May 29, 2019
lambdageek added a commit that referenced this issue May 29, 2019
when the allocation does not fit, i.e. when it is >= MONO_MEMPOOL_PREFER_INDIVIDUAL_ALLOCATION_SIZE.

Or at least make the code clearer and smaller.

"fixes" #14669.
EgorBo added a commit to EgorBo/mono that referenced this issue Jun 3, 2019
when the allocation does not fit, i.e. when it is >= MONO_MEMPOOL_PREFER_INDIVIDUAL_ALLOCATION_SIZE.

Or at least make the code clearer and smaller.

"fixes" mono#14669.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.