Issue metadata
Sign in to add a comment
|
20kb regression in resource_sizes (MonochromePublic.apk) at 479499:479499 |
||||||||||||||||||||
Issue descriptionCaused by “Return operation name in prefetch request callback and add internal page hookup” Commit: 704f4daf9c7a181b58c9d4b248dd9254baa2b4fd Link to size graph: https://chromeperf.appspot.com/report?sid=a097e74b1aa288511afb4cb616efe0f95ba4d347ad61d5e835072f23450938ba&rev=479499 Debugging size regressions is documented at: https://chromium.googlesource.com/chromium/src/+/master/tools/perf/docs/apk_size_regressions.md#Debugging-Apk-Size-Increase
,
Jun 15 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8976691584827890960
,
Jun 15 2017
=== BISECT JOB RESULTS ===
NO Perf regression found, tests failed to produce values
Bisect Details
Configuration: android_nexus7_perf_bisect
Benchmark : resource_sizes
Metric : MonochromePublic.apk_Specifics/normalized apk size
To Run This Test
src/build/android/resource_sizes.py --chromium-output-directory {CHROMIUM_OUTPUT_DIR} --chartjson {CHROMIUM_OUTPUT_DIR}/apks/MonochromePublic.apk
Debug Info
https://chromeperf.appspot.com/buildbucket_job_status/8976691584827890960
Is this bisect wrong?
https://chromeperf.appspot.com/bad_bisect?try_job_id=6639203779084288
| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
| X | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Speed>Bisection. Thank you!
,
Jun 15 2017
Looks like growth is entirely from native code. Output of tools/binary_size/diagnose_bloat.py --cloud 704f4daf9c7a181b58c9d4b248dd9254baa2b4fd: 332 symbols added (+), 10 changed (~), 2 removed (-), 486141 unchanged (not shown) 12 paths added, 0 removed, 2 changed Showing 344 symbols (285 unique) with total pss: 21475 bytes .text=16.3kb .rodata=4.00kb .data*=736 bytes .bss=48 bytes total=21.0kb Number of unique paths: 16 Index, Running Total, Section@Address, PSS ------------------------------------------------------------ + 0) 2304 (10.7%) r@0x29a1624 2304 third_party/protobuf/src/google/protobuf/stubs/structurally_valid.cc google::protobuf::internal::utf8acceptnonsurrogates ~ 1) 3596 (16.7%) r@Group 1292 {{no path}} ** merge strings (count=7) + 2) 4272 (19.9%) t@0x24c9180 676 chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc offline_internals::OfflineInternalsUIMessageHandler::HandlePrefetchRequestCallback + 3) 4860 (22.6%) t@0x256bc70 588 components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::GeneratePageBundleRequest::MergePartialFromCodedStream + 4) 5364 (25.0%) t@0x256a200 504 components/offline_pages/core/prefetch/prefetch_request_fetcher.cc offline_pages::PrefetchRequestFetcher::PrefetchRequestFetcher + 5) 5810 (27.1%) t@Group 446 components/offline_pages/core/prefetch/proto/{{shared}}/5 offline_pages::proto::MergeFromFail (count=5) + 6) 6226 (29.0%) t@0x256b728 416 components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::PageInfo::MergePartialFromCodedStream + 7) 6602 (30.7%) t@0x256c7e8 376 components/offline_pages/core/prefetch/proto/operation.pb.cc offline_pages::proto::Operation::MergePartialFromCodedStream + 8) 6970 (32.5%) t@0x256b274 368 components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::Archive::MergePartialFromCodedStream + 9) 7334 (34.1%) t@0x2569f0c 364 components/offline_pages/core/prefetch/prefetch_proto_utils.cc offline_pages::ParsePageBundleInAnyData + 10) 7694 (35.8%) t@0x74e74a 360 third_party/protobuf/src/google/protobuf/stubs/structurally_valid.cc google::protobuf::internal::UTF8GenericScan + 11) 8042 (37.4%) t@0x354848 348 base/strings/string_split.cc base::SplitStringUsingSubstr + 12) 8358 (38.9%) t@0x256cd20 316 components/offline_pages/core/prefetch/proto/status.pb.cc offline_pages::proto::Status::MergePartialFromCodedStream + 13) 8670 (40.4%) t@0x24c9048 312 chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc offline_internals::OfflineInternalsUIMessageHandler::HandleGeneratePageBundle + 14) 8970 (41.8%) t@0x25695f0 300 components/offline_pages/core/prefetch/generate_page_bundle_request.cc offline_pages::GeneratePageBundleRequest::GeneratePageBundleRequest + 15) 9256 (43.1%) t@0x74f128 286 third_party/protobuf/src/google/protobuf/wire_format_lite.cc google::protobuf::internal::WireFormatLite::VerifyUtf8String ~ 16) 9516 (44.3%) t@0x24c98d4 260 chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc offline_internals::OfflineInternalsUIMessageHandler::RegisterMessages + 17) 9772 (45.5%) r@0x29a1f29 256 third_party/protobuf/src/google/protobuf/stubs/structurally_valid.cc google::protobuf::internal::utf8acceptnonsurrogates_fast + 18) 10012 (46.6%) t@0x256bef8 240 components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::GeneratePageBundleRequest::SerializeWithCachedSizes const + 19) 10240 (47.7%) t@0x24c9424 228 chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc offline_internals::OfflineInternalsUIMessageHandler::HandleGetOperation + 20) 10460 (48.7%) t@0x256c1b0 220 components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::PageParameters::MergePartialFromCodedStream + 21) 10676 (49.7%) t@0x256ab90 216 components/offline_pages/core/prefetch/proto/any.pb.cc offline_pages::proto::Any::MergePartialFromCodedStream ~ 22) 10880 (50.7%) t@0x24c864c 204 chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc offline_internals::OfflineInternalsUIMessageHandler::HandleDeletedPagesCallback + 23) 11072 (51.6%) t@0x256adfc 192 components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::protobuf_AddDesc_offline_5fpages_2eproto_impl + 24) 11252 (52.4%) t@0x256d1dc 180 components/offline_pages/core/prefetch/proto/timestamp.pb.cc offline_pages::proto::Timestamp::MergePartialFromCodedStream + 25) 11420 (53.2%) t@0x256b56c 168 components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::PageInfo::MergeFrom + 26) 11588 (54.0%) t@0x256b970 168 components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::PageInfo::SerializeWithCachedSizes const + 27) 11746 (54.7%) t@0x2569920 158 components/offline_pages/core/prefetch/get_operation_request.cc offline_pages::GetOperationRequest::GetOperationRequest + 28) 11902 (55.4%) t@0x256af96 156 components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::PageBundle::MergePartialFromCodedStream + 29) 12052 (56.1%) t@0x256bfe8 150 components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::GeneratePageBundleRequest::ByteSize const + 30) 12200 (56.8%) t@0x74e68c 148 third_party/protobuf/src/google/protobuf/stubs/stringprintf.cc google::protobuf::StringAppendV + 31) 12348 (57.5%) t@0x2569e78 148 components/offline_pages/core/prefetch/prefetch_proto_utils.cc offline_pages::ParseOperationResponse + 32) 12496 (58.2%) t@0x256c654 148 components/offline_pages/core/prefetch/proto/operation.pb.cc offline_pages::proto::Operation::MergeFrom + 33) 12640 (58.9%) t@0x256bb14 144 components/offline_pages/core/prefetch/proto/offline_pages.pb.cc So, looks like growth is almost entirely from protobufs. +huangw, who's looking into proto bloat a bit.
,
Jun 15 2017
,
Jun 15 2017
CL 479499 did not touch any protobuf files. How could this cause the proto bloat?
,
Jun 15 2017
prefetch_proto_utils.* got updated. Maybe this lead to code inclusion that otherwise would have been optimized away (wild guess)?
,
Jun 15 2017
Perhaps you're now calling a function that was being dead-code-eliminated before? All the protos mention "offline" in them, so I believe the diff to be accurate.
,
Jun 15 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8976687251518643584
,
Jun 15 2017
=== BISECT JOB RESULTS ===
NO Perf regression found, tests failed to produce values
Bisect Details
Configuration: android_nexus7_perf_bisect
Benchmark : resource_sizes
Metric : MonochromePublic.apk_Specifics/normalized apk size
To Run This Test
src/build/android/resource_sizes.py --chromium-output-directory {CHROMIUM_OUTPUT_DIR} --chartjson {CHROMIUM_OUTPUT_DIR}/apks/MonochromePublic.apk
Debug Info
https://chromeperf.appspot.com/buildbucket_job_status/8976687251518643584
Is this bisect wrong?
https://chromeperf.appspot.com/bad_bisect?try_job_id=6639203779084288
| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
| X | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Speed>Bisection. Thank you!
,
Jun 15 2017
Before this CL, all offline prefetch proto files and their consumers are only referenced from test files. With this CL, we now have an entry in offline internals page. This is the reason we suddenly see the size increase. haungs, do you know anything we can do to optimize? Also, is there a way I can run the analysis command locally against apk file so that I can test various things I can do to improve it?
,
Jun 15 2017
Looks like prefetech protos "option optimize_for = LITE_RUNTIME;". Try CODE_SIZE to see if it makes any difference?? To run locally, documentation: https://chromium.googlesource.com/chromium/src/+/master/tools/binary_size From my notes I ran these; maybe these can get you started: ******** To Generate size file ******** tools/binary_size/supersize archive chrome.size --apk-file out/Release/apks/Chrome.apk -v ******** To Categorize (for chrome.size) ******** tools/binary_size/supersize console chrome.size --query 'Print(canned_queries.CategorizeGenerated())' ******** To Extract Protocol Buffer related stuff (for chrome.size) ******** tools/binary_size/supersize console chrome.size --query 'Print(size_info.symbols.WhereObjectPathMatches(".*/protobuf/|\\.pb\\.o$|\\.pbzero\\.o$").Sorted(),verbose=1)'
,
Jun 16 2017
As stated in the readme, there's also a tool called "diagnose_bloat.py", which wraps supersize by taking care of building before & after your commit as well as setting appropriate GN args.
,
Jun 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/08ca13d0991463f5646fb88a19460ea2b2778d14 commit 08ca13d0991463f5646fb88a19460ea2b2778d14 Author: jianli <jianli@chromium.org> Date: Fri Jun 16 23:58:05 2017 [Offline Prefetch] Use proto2 syntax for offline prefetch proto Switching back to proto2 greatly reduced the size. Also moving the proto definitions in timestamp.proto to where are referenced also saved a bunch. Note that moving other proto files did not save much. BUG= 733783 Review-Url: https://codereview.chromium.org/2940393002 Cr-Commit-Position: refs/heads/master@{#480230} [modify] https://crrev.com/08ca13d0991463f5646fb88a19460ea2b2778d14/components/offline_pages/core/prefetch/BUILD.gn [modify] https://crrev.com/08ca13d0991463f5646fb88a19460ea2b2778d14/components/offline_pages/core/prefetch/proto/any.proto [modify] https://crrev.com/08ca13d0991463f5646fb88a19460ea2b2778d14/components/offline_pages/core/prefetch/proto/offline_pages.proto [modify] https://crrev.com/08ca13d0991463f5646fb88a19460ea2b2778d14/components/offline_pages/core/prefetch/proto/operation.proto [modify] https://crrev.com/08ca13d0991463f5646fb88a19460ea2b2778d14/components/offline_pages/core/prefetch/proto/status.proto [delete] https://crrev.com/1ff5a375ab0b3c53a6893a1e257c2d81a16a485b/components/offline_pages/core/prefetch/proto/timestamp.proto
,
Jun 19 2017
Looks like the CL in c#14 reduced the normalized apk size by about 4 kb (graph here: https://chromeperf.appspot.com/report?sid=016de86be5b75b47c5aff0964454f0f9799da60f0c38188f9863c37f1072f40f&rev=480230). Index | Running Total | Section@Address | Δ PSS (Δ size_without_padding) | Path ------------------------------------------------------------ - 0) -2304 (62.6%) r@0x0 -2304 (2304->0) third_party/protobuf/src/google/protobuf/stubs/structurally_valid.cc google::protobuf::internal::utf8acceptnonsurrogates ~ 1) -2811 (76.3%) r@Group -507 (2968883->2968376) {{no path}} ** merge strings (count=7) - 2) -3171 (86.1%) t@0x0 -360 (360->0) third_party/protobuf/src/google/protobuf/stubs/structurally_valid.cc google::protobuf::internal::UTF8GenericScan - 3) -3457 (93.9%) t@0x0 -286 (284->0) third_party/protobuf/src/google/protobuf/wire_format_lite.cc google::protobuf::internal::WireFormatLite::VerifyUtf8String - 4) -3713 (100.8%) r@0x0 -256 (256->0) third_party/protobuf/src/google/protobuf/stubs/structurally_valid.cc google::protobuf::internal::utf8acceptnonsurrogates_fast - 5) -3861 (104.8%) t@0x0 -148 (148->0) third_party/protobuf/src/google/protobuf/stubs/stringprintf.cc google::protobuf::StringAppendV - 6) -3991 (108.3%) t@0x0 -130 (130->0) third_party/protobuf/src/google/protobuf/stubs/structurally_valid.cc google::protobuf::internal::UTF8GenericScanFastAscii ~ 7) -4087 (111.0%) t@Group -96 (0->0) {{no path}} ** symbol gaps (count=9) - 8) -4177 (113.4%) t@0x0 -90 (88->0) components/offline_pages/core/prefetch/proto/{{shared}}/5 offline_pages::proto::MergeFromFail - 9) -4265 (115.8%) t@0x0 -88 (88->0) components/offline_pages/core/prefetch/proto/timestamp.pb.cc offline_pages::proto::protobuf_AddDesc_timestamp_2eproto_impl ~ 10) -4347 (118.0%) t@0x2580c18 -82 (240->158) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::GeneratePageBundleRequest::SerializeWithCachedSizes const ~ 11) -4275 (116.1%) t@0x257f85c +72 (36->108) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::Timestamp::MergeFrom ~ 12) -4344 (117.9%) t@0x22f648c -69 (92->68) components/offline_pages/core/prefetch/proto/any.pb.cc offline_pages::proto::Any::SerializeWithCachedSizes const (num_aliases=1->3) ~ 13) -4413 (119.8%) t@0x191a5d4 -68 (80->68) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::PageParameters::SerializeWithCachedSizes const (num_aliases=1->6) ~ 14) -4345 (118.0%) t@0x257fd64 +68 (92->160) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::Archive::MergeFrom ~ 15) -4277 (116.1%) t@0x257f930 +68 (180->248) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::Timestamp::MergePartialFromCodedStream ~ 16) -4217 (114.5%) t@0x25807e4 +60 (144->204) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::GeneratePageBundleRequest::MergeFrom ~ 17) -4157 (112.9%) t@0x2580d90 +60 (64->124) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::PageParameters::MergeFrom ~ 18) -4214 (114.4%) t@0x18167f0 -57 (60->68) components/offline_pages/core/prefetch/proto/any.pb.cc offline_pages::proto::Any::ByteSize const (num_aliases=1->28) ~ 19) -4270 (115.9%) t@0x25809b8 -56 (588->532) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::GeneratePageBundleRequest::MergePartialFromCodedStream ~ 20) -4214 (114.4%) t@0x257fb90 +56 (156->212) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::PageBundle::MergePartialFromCodedStream + 21) -4268 (115.9%) t@0x0 -53 (0->0.0) {{no path}} ** aggregate padding of diff'ed symbols ~ 22) -4216 (114.5%) t@0x257fed4 +52 (368->420) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::Archive::MergePartialFromCodedStream ~ 23) -4164 (113.1%) t@0x25812f8 +52 (148->200) components/offline_pages/core/prefetch/proto/operation.pb.cc offline_pages::proto::Operation::MergeFrom ~ 24) -4216 (114.5%) t@0x2580678 -52 (168->116) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::PageInfo::SerializeWithCachedSizes const ~ 25) -4164 (113.1%) t@0x2581954 +52 (76->128) components/offline_pages/core/prefetch/proto/status.pb.cc offline_pages::proto::Status::MergeFrom ~ 26) -4216 (114.5%) t@0x257f79c -52 (192->140) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::protobuf_AddDesc_offline_5fpages_2eproto_impl - 27) -4264 (115.8%) d@0x0 -48 (48->0) third_party/protobuf/src/google/protobuf/stubs/structurally_valid.cc google::protobuf::internal::utf8acceptnonsurrogates_obj ~ 28) -4216 (114.5%) t@0x2580948 +48 (64->112) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::GeneratePageBundleRequest::Clear ~ 29) -4168 (113.2%) t@0x25801f4 +48 (168->216) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::PageInfo::MergeFrom ~ 30) -4120 (111.9%) t@0x2580e80 +48 (220->268) components/offline_pages/core/prefetch/proto/offline_pages.pb.cc offline_pages::proto::PageParameters::MergePartialFromCodedStream ~ 31) -4076 (110.7%) t@0x257f490 +44 (92->136) components/offline_pages/core/prefetch/proto/any.pb.cc offline_pages::proto::Any::MergeFrom - 32) -4118 (111.8%) t@0x0 -42 (42->0) third_party/protobuf/src/google/protobuf/stubs/stringprintf.cc google::protobuf::StringPrintf Is there anything else we can do here? Otherwise feel free to close as fixed. Thanks for looking into it!
,
Jun 19 2017
Silly question: Aside from switching from Proto 3 to Proto 2, you're also adding many "optional" lines. Can optional be added for Proto 3??
,
Jun 19 2017
optional is not needed in proto3 syntax.
,
Jun 29 2017
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by agrieve@chromium.org
, Jun 15 2017