Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1057)

Issue 326793002: Reduce the size of Node by 8 bytes on 64bit with gcc/clang compilers (Closed)

Created:
6 years, 6 months ago by Inactive
Modified:
6 years, 6 months ago
Reviewers:
esprehn
CC:
blink-reviews, blink-reviews-dom_chromium.org, dglazkov+blink, eae+blinkwatch, rwlbuis, sof
Visibility:
Public.

Description

Reduce the size of Node by 8 bytes on 64bit with gcc/clang compilers Reduce the size of Node by 8 bytes on 64bit with gcc and clang compilers (from 72 bytes to 64 bytes) by altering the order of its parent classes. The TreeShared<Node> parent class now comes last because it contains only 1 member (in release) which is an int and the first member of Node is a uint32_t. This way no padding is added by gcc/clang and these two members will use the same word in memory. MSVC likely won't get any benefit from this because it makes sure each class in the hierarchy is memory-aligned (see "Compiler being used" section in [1]). There should be no size impact for MSVC. [1] http://www.cprogramming.com/tutorial/size_of_class_object.html R=esprehn@chromium.org Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175844

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M Source/core/dom/Node.h View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 7 (0 generated)
Inactive
6 years, 6 months ago (2014-06-10 00:02:22 UTC) #1
esprehn
What does clang do?
6 years, 6 months ago (2014-06-10 00:15:06 UTC) #2
Inactive
On 2014/06/10 00:15:06, esprehn wrote: > What does clang do? I have just tried clang, ...
6 years, 6 months ago (2014-06-10 00:41:56 UTC) #3
esprehn
lgtm
6 years, 6 months ago (2014-06-10 00:46:57 UTC) #4
Inactive
The CQ bit was checked by ch.dumez@samsung.com
6 years, 6 months ago (2014-06-10 00:52:25 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/326793002/1
6 years, 6 months ago (2014-06-10 00:52:51 UTC) #6
commit-bot: I haz the power
6 years, 6 months ago (2014-06-10 01:28:35 UTC) #7
Message was sent while issue was closed.
Change committed as 175844

Powered by Google App Engine
This is Rietveld 408576698