User-Agent: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.7) Gecko/2009021906 Firefox/3.0.7 Build Identifier: FixedMalloc::Alloc aligns the requested size to 4 bytes. This should either be 8 bytes or the alignment code should be removed since the underlying functions - FindSizeClass and SizeToBlocks align the requests to 8 bytes. Reproducible: Always

For 32 bit platforms I think its important to keep the 4 byte bucket, there is code in FindSizeClass which causes index 0 to be used.

(In reply to comment #1) > For 32 bit platforms I think its important to keep the 4 byte bucket, there is > code in FindSizeClass which causes index 0 to be used. Agree, this point was also made (though in a weaker sense) in comment 11 on https://bugzilla.mozilla.org/show_bug.cgi?id=481683. Since we're talking about it, I'm interested in knowing whether you have any idea where allocs of size 4 originate? I can imagine situations for it - abstract data types that turn out to be only 4 bytes when all is said and done, for example - but I wonder what you've seen that makes this a compelling size class.

The player's StrAlloc API mostly, this is the generic C string alloc routine and its mapped to FixedMalloc (or more accurately new char[]). We do size class based dumping in GCAlloc, it would be nice to do it for FixedMalloc too (show per size class stats), I'll make a bug.

I think that the alignment to 4 code in FixedMalloc::Alloc (FixedMalloc.h) is not required altogether. The aligned size when passed to FindSizeClass is mapped to an index based on whether it is greater than 4 or not. uint32_t size8 = (uint32_t)((size+7)&~7); // round up to multiple of 8#ifdef MMGC_64BIT unsigned index = size8 ? ((size8 >> 3) - 1) : 0; #else unsigned index = size > 4 ? size8 >> 3 : 0; #endif So, size = 0 - 4 bytes --> Index = 0 size > 4 bytes --> Index = ((size8) >> 3) It doesn't matter if the size argument passed to FindSizeClass is 1, 2, 3, or 4 bytes. So I think the caller need not perform any byte alignment on size. For largeAllocs, GCHeap::SizeToBlocks() function aligns it to the kBlockSize so that case is taken care of as well. Is that right or am I missing something?

sounds good to me, we should be passing the un-adultered size to the profiling AllocHook so removing the alignment in Alloc will make the profiler fragmentation tracking more honest

Fixed changeset: 1687:81823140cd70