Bug 47921 - Make NodeRareData::rareDataMap a non-inline function
Summary: Make NodeRareData::rareDataMap a non-inline function
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Justin Schuh
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-19 13:12 PDT by Justin Schuh
Modified: 2010-10-19 17:18 PDT (History)
1 user (show)

See Also:


Attachments
Patch (10.51 KB, patch)
2010-10-19 13:21 PDT, Justin Schuh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Schuh 2010-10-19 13:12:06 PDT
Make NodeRareData::rareDataMap a non-inline function
Comment 1 Justin Schuh 2010-10-19 13:21:33 PDT
Created attachment 71196 [details]
Patch

NodeRareData::rareDataMap is an inline function with a static local variable. So, it will be unnecessarily duplicated in each compilation unit. It should be moved to a source file.
Comment 2 Alexey Proskuryakov 2010-10-19 13:33:37 PDT
How did you assess performance implications of this change?

> So, it will be unnecessarily duplicated in each compilation unit.

That's a burden on the linker, but should be fine otherwise.
Comment 3 Justin Schuh 2010-10-19 16:51:24 PDT
(In reply to comment #2)
> How did you assess performance implications of this change?

I hadn't considered it in terms of performance. I was looking at it from the perspective of having multiple, different copies of dataMap.

> > So, it will be unnecessarily duplicated in each compilation unit.
> 
> That's a burden on the linker, but should be fine otherwise.

Unless I'm confused, won't the compiler generate a separate copy of the dataMap static variable for every source file that includes NodeRareData.h and calls NodeRareData::rareDataMap?
Comment 4 Justin Schuh 2010-10-19 17:02:28 PDT
Comment on attachment 71196 [details]
Patch

And after a little poking around it appears I was confused. Sorry for the hassle.
Comment 5 Alexey Proskuryakov 2010-10-19 17:18:41 PDT
Yes, the compiler will generate multiple copies - but then the linker will merge them all (I'm pretty sure this works with any reasonable toolchain).

I'm not sure how much effect this has on linking performance, but it doesn't affect correctness.