Issue metadata
Sign in to add a comment
|
32kb regression in resource_sizes (MonochromePublic.apk) at 564422:564422 |
||||||||||||||||||||
Issue descriptionCaused by “v8binding: Moves ExceptionCode into platform/.” Commit: 19b6f8060fb08b2b74eb4fe1723658b80108b906 Link to size graph: https://chromeperf.appspot.com/report?sid=a097e74b1aa288511afb4cb616efe0f95ba4d347ad61d5e835072f23450938ba&num_points=10&rev=564422 Debugging size regressions is documented at: https://chromium.googlesource.com/chromium/src/+/master/docs/speed/apk_size_regressions.md#Debugging-Apk-Size-Increase Based on the graph: 32kb native code It's not clear to me whether or not this increase was expected. Please have a look and either: - Close as “Won't Fix” with a short justification, or - Land a revert / fix-up.
,
Jun 5 2018
Assigning to yukishiino@chromium.org because this is the only CL in range: v8binding: Moves ExceptionCode into platform/. As part of effort to move ExceptionState into platform/, this patch moves ExceptionCode into platform/. Bug: 849607 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I015f91037dab37c5c3bc83e77ed6e68a2b6251fd Reviewed-on: https://chromium-review.googlesource.com/1084576 Commit-Queue: Yuki Shiino <yukishiino@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#564422}
,
Jun 5 2018
It looks like around 2000 symbols grew a small amount (1-100 bytes each). Here are the top most changed symbols. I have also attached the full diff.
Section Legend: t=.text, r=.rodata, R=.data.rel.ro, d=.data, b=.bss, x=.dex, m=.dex.method, p=.pak.translations, P=.pak.nontranslated, o=.other
Index | Running Total | Section@Address | Δ PSS (Δ size_without_padding) | Path
------------------------------------------------------------
+ 0) 5480 (16.7%) t@0x21e357c 5480 (0->5480) $root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_webgl2_rendering_context.cc
blink::WebGL2RenderingContextV8Internal::texSubImage2DMethod
~ 1) 4 (0.0%) t@0x21e3578 -5476 (5480->4) third_party/blink/renderer/bindings/modules/v8/v8_webgl2_rendering_context.cc
blink::V8WebGL2RenderingContext::texSubImage2DMethodCallback
~ 2) -5284 (-16.1%) t@0x21e20cc -5288 (5292->4) third_party/blink/renderer/bindings/modules/v8/v8_webgl2_rendering_context.cc
blink::V8WebGL2RenderingContext::texImage2DMethodCallback
+ 3) 4 (0.0%) t@0x21e20d0 5288 (0->5288) $root_gen_dir/third_party/blink/renderer/bindings/modules/v8/v8_webgl2_rendering_context.cc
blink::WebGL2RenderingContextV8Internal::texImage2DMethod
~ 4) -564 (-1.7%) o@0x0 -568 (0->0) {no path}
Overhead: ELF file
~ 5) -324 (-1.0%) t@Group 240 (79144->79384) {no path}
** thunk (count=2)
~ 6) -208 (-0.6%) t@0x1ef9578 116 (672->788) third_party/blink/renderer/core/html/canvas/canvas_rendering_context_host.cc
blink::CanvasRenderingContextHost::convertToBlob const
~ 7) -92 (-0.3%) t@0x227fd84 116 (1248->1364) third_party/blink/renderer/modules/indexeddb/idb_object_store.cc
blink::IDBObjectStore::DoPut
~ 8) 16 (0.0%) t@0x1dd1980 108 (488->596) third_party/blink/renderer/core/css/property_registration.cc
blink::PropertyRegistration::registerProperty
~ 9) 104 (0.3%) t@0x1d4c720 88 (3400->3488) third_party/blink/renderer/core/animation/effect_input.cc
blink::EffectInput::ParseKeyframesArgument
~ 10) 192 (0.6%) t@0x22b40e8 88 (1452->1540) third_party/blink/renderer/modules/payments/payment_request.cc
blink::PaymentRequest::PaymentRequest
~ 11) 276 (0.8%) t@0x224c9fc 84 (808->892) third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc
blink::BaseRenderingContext2D::getImageData
~ 12) 360 (1.1%) t@0x2279988 84 (460->544) third_party/blink/renderer/modules/indexeddb/idb_cursor.cc
blink::IDBCursor::continuePrimaryKey
~ 13) 444 (1.4%) t@0x22b5ab8 84 (920->1004) third_party/blink/renderer/modules/payments/payment_request.cc
blink::StringifyAndParseMethodSpecificData
~ 14) 520 (1.6%) t@0x223aa28 76 (1152->1228) third_party/blink/renderer/modules/bluetooth/bluetooth.cc
blink::Bluetooth::requestDevice
~ 15) 584 (1.8%) t@0x228d2c4 64 (568->632) third_party/blink/renderer/modules/locks/lock_manager.cc
blink::LockManager::request
~ 16) 644 (2.0%) t@0x1d79a2c 60 (424->484) third_party/blink/renderer/core/css/css_style_sheet.cc
blink::CSSStyleSheet::insertRule
~ 17) 704 (2.1%) t@0x225071c 60 (376->436) third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.cc
blink::CanvasRenderingContext2D::addHitRegion
~ 18) 648 (2.0%) t@0x224cd98 -56 (840->784) third_party/blink/renderer/modules/canvas/canvas2d/base_rendering_context_2d.cc
blink::BaseRenderingContext2D::putImageData
~ 19) 704 (2.1%) t@0x21a3558 56 (896->952) third_party/blink/renderer/bindings/modules/v8/v8_lock_manager.cc
blink::V8LockManager::requestMethodCallback
~ 20) 756 (2.3%) t@0x229a19c 52 (264->316) third_party/blink/renderer/modules/mediacapturefromelement/html_canvas_element_capture.cc
blink::HTMLCanvasElementCapture::captureStream
~ 21) 808 (2.5%) t@0x228079c 52 (720->772) third_party/blink/renderer/modules/indexeddb/idb_object_store.cc
blink::IDBObjectStore::createIndex
~ 22) 860 (2.6%) t@0x1fd44e0 52 (664->716) third_party/blink/renderer/core/intersection_observer/intersection_observer.cc
blink::IntersectionObserver::Create
~ 23) 912 (2.8%) t@0x22c6bc0 52 (456->508) third_party/blink/renderer/modules/presentation/presentation_request.cc
blink::PresentationRequest::Create
~ 24) 964 (2.9%) t@0x1eb0f9c 52 (1960->2012) third_party/blink/renderer/core/fetch/request.cc
blink::Request::CreateRequestWithRequestOrString
~ 25) 1016 (3.1%) t@0x216f300 52 (552->604) third_party/blink/renderer/bindings/modules/v8/v8_payment_details_modifier.cc
blink::V8PaymentDetailsModifier::ToImpl
~ 26) 1068 (3.3%) t@0x216fc14 52 (360->412) third_party/blink/renderer/bindings/modules/v8/v8_payment_method_data.cc
******************************Resource Sizes Diff******************************
MonochromePublic.apk_Breakdown (+32,770 bytes)
+18 bytes Zip Overhead
-18 bytes unwind_cfi (dev and canary only) size
+32,768 bytes Native code size
+2 bytes Package metadata size
MonochromePublic.apk_Specifics
+32,788 bytes normalized apk size
+32,768 bytes main lib size
,
Jun 6 2018
My patch encloses existing global enum values into a namespace (actually it's a class scope, not C++ namespace, though).
kV8TypeError ==> ESErrorType::kTypeError
so, some amount of the increase of the symbol table is reasonable (I expected enums not to have a symbol table, though).
I'm planning to make a class-scope into a enum class (scoped enumeration). If class-scope made this regression and enum class doesn't use a symbol table (maybe not true), then this regression will be resolved then.
,
Jun 6 2018
I don't think that is the reason. After some discussion with agrieve@, we believe that the cause of the jump is you removing one of the overloads for blink::ExceptionState::Throw{DOMException, RangeError, SecurityError, TypeError} that takes a c string and left only one that takes String.
Which basically means that the conversion that used to happen in the overloaded implementation (from c string to String) is now inlined at every call site increasing the code size. I tested this by reverting you change and only removing the overloads and the code size by a similar amount as this CL.
Please add the overloads back in.
,
Jun 7 2018
Thanks for the investigation! Yes, I will.
,
Jun 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5250668dd25202dbdfeb980e180b809b2781e821 commit 5250668dd25202dbdfeb980e180b809b2781e821 Author: Yuki Shiino <yukishiino@chromium.org> Date: Thu Jun 07 14:27:03 2018 v8binding: Reduce the binary code size of ExceptionState's call sites. https://crrev.com/c/1084576 increased the binary code size by removing overloads of |const char*| version in ExceptionState. This patch simply adds them back with an explanation why we need them. Bug: 849743 Change-Id: I634759c1aa514d5199f5b712336db8acffa5257a Reviewed-on: https://chromium-review.googlesource.com/1090691 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Hitoshi Yoshida <peria@chromium.org> Commit-Queue: Yuki Shiino <yukishiino@chromium.org> Cr-Commit-Position: refs/heads/master@{#565259} [modify] https://crrev.com/5250668dd25202dbdfeb980e180b809b2781e821/third_party/blink/renderer/bindings/core/v8/exception_state.cc [modify] https://crrev.com/5250668dd25202dbdfeb980e180b809b2781e821/third_party/blink/renderer/bindings/core/v8/exception_state.h
,
Jun 8 2018
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Jun 5 2018