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

Issue 2843293003: Optimized layout padding in 4 classes in ast.h (Closed)

Created:
3 years, 7 months ago by stanisc
Modified:
3 years, 7 months ago
Reviewers:
adamk
CC:
v8-reviews_googlegroups.com
Target Ref:
refs/heads/master
Project:
v8
Visibility:
Public.

Description

Optimized layout padding in 4 classes in ast.h This reduces sizeof of these classes by 8 bytes on 64-bit (16 bytes considering allocation size granularity for some of these classes). I don't know how many instances remain at the end of loading a page. These objects are Zone objects which makes it more difficult to count the number of instances. But looking at allocations only on cnn.com I've got 70K for BinaryOperation, 20K for CompareOperation, 1.5K for CaseClause. There aren't not many allocations of NativeFunctionLiteral but I decided to fix it too to keep the same layout pattern. Before: class v8::internal::CaseClause [sizeof = 56] : public v8::internal::Expression { [sizeof=12] v8::internal::Expression <padding> (4 bytes) [sizeof=8] v8::internal::Expression* label_ [sizeof=8] v8::internal::Label body_target_ [sizeof=8] v8::internal::ZoneList<v8::internal::Statement *>* statements_ [sizeof=8] v8::internal::AstType* compare_type_ [sizeof=4] v8::internal::FeedbackSlot feedback_slot_ <padding> (4 bytes) } After: class v8::internal::CaseClause [sizeof = 48] : public v8::internal::Expression { [sizeof=12] v8::internal::Expression [sizeof=4] v8::internal::FeedbackSlot feedback_slot_ [sizeof=8] v8::internal::Expression* label_ [sizeof=8] v8::internal::Label body_target_ [sizeof=8] v8::internal::ZoneList<v8::internal::Statement *>* statements_ [sizeof=8] v8::internal::AstType* compare_type_ } Before: class v8::internal::BinaryOperation [sizeof = 56] : public v8::internal::Expression { [sizeof=12] v8::internal::Expression [sizeof=1] bool has_fixed_right_arg_ <padding> (3 bytes) [sizeof=4] int fixed_right_arg_value_ <padding> (4 bytes) [sizeof=8] v8::internal::Expression* left_ [sizeof=8] v8::internal::Expression* right_ [sizeof=8] v8::internal::Handle<v8::internal::AllocationSite> allocation_site_ [sizeof=4] v8::internal::FeedbackSlot feedback_slot_ <padding> (4 bytes) } After: class v8::internal::BinaryOperation [sizeof = 48] : public v8::internal::Expression { [sizeof=12] v8::internal::Expression [sizeof=4] v8::internal::FeedbackSlot feedback_slot_ [sizeof=8] v8::internal::Expression* left_ [sizeof=8] v8::internal::Expression* right_ [sizeof=8] v8::internal::Handle<v8::internal::AllocationSite> allocation_site_ [sizeof=1] bool has_fixed_right_arg_ <padding> (3 bytes) [sizeof=4] int fixed_right_arg_value_ } Before: class v8::internal::CompareOperation [sizeof = 48] : public v8::internal::Expression { [sizeof=12] v8::internal::Expression <padding> (4 bytes) [sizeof=8] v8::internal::Expression* left_ [sizeof=8] v8::internal::Expression* right_ [sizeof=8] v8::internal::AstType* combined_type_ [sizeof=4] v8::internal::FeedbackSlot feedback_slot_ <padding> (4 bytes) } After: class v8::internal::CompareOperation [sizeof = 40] : public v8::internal::Expression { [sizeof=12] v8::internal::Expression [sizeof=4] v8::internal::FeedbackSlot feedback_slot_ [sizeof=8] v8::internal::Expression* left_ [sizeof=8] v8::internal::Expression* right_ [sizeof=8] v8::internal::AstType* combined_type_ } Before: class v8::internal::NativeFunctionLiteral [sizeof = 40] : public v8::internal::Expression { [sizeof=12] v8::internal::Expression <padding> (4 bytes) [sizeof=8] v8::internal::AstRawString* name_ [sizeof=8] v8::Extension* extension_ [sizeof=4] v8::internal::FeedbackSlot literal_feedback_slot_ <padding> (4 bytes) } After: class v8::internal::NativeFunctionLiteral [sizeof = 32] : public v8::internal::Expression { [sizeof=12] v8::internal::Expression [sizeof=4] v8::internal::FeedbackSlot literal_feedback_slot_ [sizeof=8] v8::internal::AstRawString* name_ [sizeof=8] v8::Extension* extension_ } BUG=chromium:710933 Review-Url: https://codereview.chromium.org/2843293003 Cr-Commit-Position: refs/heads/master@{#44989} Committed: https://chromium.googlesource.com/v8/v8/+/6408032e6164f7f1b2f9edab75d1b14e4cbb32e4

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -11 lines) Patch
M src/ast/ast.h View 5 chunks +11 lines, -11 lines 0 comments Download

Messages

Total messages: 15 (11 generated)
stanisc
PTAL!
3 years, 7 months ago (2017-04-28 18:51:32 UTC) #8
adamk
lgtm, thanks! One note on V8 changes: when listing an attached bug, please prefix with ...
3 years, 7 months ago (2017-04-28 20:53:10 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2843293003/1
3 years, 7 months ago (2017-04-28 20:57:39 UTC) #12
commit-bot: I haz the power
3 years, 7 months ago (2017-04-28 20:59:05 UTC) #15
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/v8/v8/+/6408032e6164f7f1b2f9edab75d1b14e4cb...

Powered by Google App Engine
This is Rietveld 408576698