Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(21)

Issue 2755323002: [Bindings] Emit less code per member in generated dictionary ToV8. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 months, 1 week ago by jbroman
Modified:
6 months, 1 week ago
Reviewers:
haraken
CC:
blink-reviews, blink-reviews-bindings_chromium.org, chromium-reviews
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Bindings] Emit less code per member in generated dictionary ToV8. Currently, we emit two invocations of CreateDataProperty for members that are defaulted or nullable. Since the invocation and handling is identical in all cases where we store a value, we can emit the code to do so just once. This reduces Android APK size by 16 KiB. The difference in specific symbols is can be fairly dramatic, with code size of conversion methods reduced by up to 35%. Review-Url: https://codereview.chromium.org/2755323002 Cr-Commit-Position: refs/heads/master@{#457998} Committed: https://chromium.googlesource.com/chromium/src/+/0ff10ccfef86c660f344077896fb93ebea341ec8

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+402 lines, -128 lines) Patch
M third_party/WebKit/Source/bindings/templates/dictionary_v8.cpp.tmpl View 1 chunk +16 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestDictionary.cpp View 1 chunk +336 lines, -108 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestDictionaryDerived.cpp View 1 chunk +34 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceEventInit.cpp View 1 chunk +8 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/bindings/tests/results/core/V8TestPermissiveDictionary.cpp View 1 chunk +8 lines, -2 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 16 (12 generated)
jbroman
6 months, 1 week ago (2017-03-19 05:02:41 UTC) #10
haraken
LGTM
6 months, 1 week ago (2017-03-19 05:59:40 UTC) #11
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/2755323002/1
6 months, 1 week ago (2017-03-19 18:11:04 UTC) #13
commit-bot: I haz the power
6 months, 1 week ago (2017-03-19 18:16:32 UTC) #16
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/0ff10ccfef86c660f344077896fb...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b