[PATCH] Allocate MCSymbol Name intrusively only when required
Duncan P. N. Exon Smith
dexonsmith at apple.com
Mon Jun 8 13:45:09 PDT 2015
> On 2015 Jun 8, at 13:16, Pete Cooper <peter_cooper at apple.com> wrote:
>
>
>> On Jun 8, 2015, at 1:11 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>> On Mon, Jun 8, 2015 at 12:19 PM, Pete Cooper <peter_cooper at apple.com> wrote:
>> Hi all
>>
>> Recent work by Rafael and Duncan has greatly reduced the number of MCSymbol’s which are actually named. In particular, temporary symbols typically don’t need names. On the verify_uselist_order test case for example, only around 50,000 symbols have names out of 700,000 total symbols.
>>
>> This patch moves the storage for the Name* itself to be allocated prior to the MCSymbol.
>>
>> Any particular reason it goes before rather than after? (I know some of our data structures put things after)
> There’s a couple of subclasses of MCSymbol - MCSymbolCOFF and MCSymbolELF - so i thought it would be tricker to put it after those.
>>
>> There's no need to reinterpret_cast to MCSymbol when you just want a void* in operator new.
> Good catch. Thanks. I’ll fix that.
FWIW, I've found the opposite in a few places in our code base.
Sometimes you need to cast to `void *` to disambiguate from other
versions of `operator new()`.
>>
>> The alignment of the allocation might be interesting - given that it needs to be aligned as much as a pointer, not just MCSymbol (I assume MCSymbol is at least as aligned as a pointer right now, so you probably get this for free/correct without explicitly accounting for it)
> Thats true. There are currently pointers in there, but good to be explicit about it. I’ll fix that too.
>>
>> The extra indirection hidden behind NameTy is a bit confusing (especially since you have pointers to it - so it's pointers on pointers).
> Yeah, I see what you mean. I’ll move the * out of NameTy.
>
> Thanks for the comments. I’ll send out an updated patch shortly.
>
> Cheers,
> Pete
>>
>> This is similar to how User allocates Use’s in a custom new/delete.
>>
>> Given that a symbol is currently 48 bytes on MachO, this reduces it by 16% on the typical case. I have other patches coming which will get us to 24-bytes which is on average half the current amount.
>>
>> Cheers,
>> Pete
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>
More information about the llvm-commits
mailing list