0.2% installer size (OSX) regression on Tue October 25 2022
Categories
(Firefox :: Sync, defect)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox-esr102 | --- | unaffected |
| firefox106 | --- | unaffected |
| firefox107 | --- | unaffected |
| firefox108 | --- | wontfix |
| firefox109 | --- | wontfix |
People
(Reporter: aesanu, Assigned: skhamis)
References
(Regression)
Details
(Keywords: perf-alert, regression, Whiteboard: [fxsync-])
Perfherder has detected a build_metrics performance regression from push 11b26634038d03b2f0ffe4fcf7efa6eec5979847. As author of one of the patches included in that push, we need your help to address this regression.
Regressions:
| Ratio | Test | Platform | Options | Absolute values (old vs new) |
|---|---|---|---|---|
| 0.20% | installer size | osx-shippable | instrumented | 121,541,911.08 -> 121,778,961.25 |
Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests. Please follow our guide to handling regression bugs and let us know your plans within 3 business days, or the offending patch(es) may be backed out in accordance with our regression policy.
For more information on performance sheriffing please see our FAQ.
|
||
Comment 1•2 years ago
|
||
Set release status flags based on info from the regressing bug 1791851
|
Assignee | |
Updated•2 years ago
|
|
|
Assignee | |
Updated•2 years ago
|
|
|
Assignee | |
Comment 2•2 years ago
|
||
Hi Andrea,
I would like to identify a bit more what exactly is going on here (a bit surprised this caused an installer size regression).
-
Is it possible to run these tests locally? I was trying to look at the subtests to run in the alert summary but didn't see what specific tests I could try to get me to run this
-
If this is a try-only type of test, I'd like to compare the stack of diffs against each other:
- https://hg.mozilla.org/integration/autoland/rev/2b85dacb39005fcaf5f8814a91b0db5220f0c570 vs https://hg.mozilla.org/integration/autoland/rev/6b591feffdb3b583d005b551fa76d9fe63d8fb28
- https://hg.mozilla.org/integration/autoland/rev/6b591feffdb3b583d005b551fa76d9fe63d8fb28 vs https://hg.mozilla.org/integration/autoland/rev/5667db8fed85d3c78a9822d22c4d40fe8da93e95
- https://hg.mozilla.org/integration/autoland/rev/5667db8fed85d3c78a9822d22c4d40fe8da93e95 vs https://hg.mozilla.org/integration/autoland/rev/11b26634038d03b2f0ffe4fcf7efa6eec5979847
To actually see which commit in the stack caused the regression.
Thanks!
|
Reporter | |
Comment 3•2 years ago
|
||
(In reply to Sammy Khamis [:skhamis] from comment #2)
Hi Andrea,
I would like to identify a bit more what exactly is going on here (a bit surprised this caused an installer size regression).
Is it possible to run these tests locally? I was trying to look at the subtests to run in the alert summary but didn't see what specific tests I could try to get me to run this
If this is a try-only type of test, I'd like to compare the stack of diffs against each other:
- https://hg.mozilla.org/integration/autoland/rev/2b85dacb39005fcaf5f8814a91b0db5220f0c570 vs https://hg.mozilla.org/integration/autoland/rev/6b591feffdb3b583d005b551fa76d9fe63d8fb28
- https://hg.mozilla.org/integration/autoland/rev/6b591feffdb3b583d005b551fa76d9fe63d8fb28 vs https://hg.mozilla.org/integration/autoland/rev/5667db8fed85d3c78a9822d22c4d40fe8da93e95
- https://hg.mozilla.org/integration/autoland/rev/5667db8fed85d3c78a9822d22c4d40fe8da93e95 vs https://hg.mozilla.org/integration/autoland/rev/11b26634038d03b2f0ffe4fcf7efa6eec5979847
To actually see which commit in the stack caused the regression.
Thanks!
Hi,
You can use only the hashes from the revisions, select the project to be autoland and the framework to be build_metrics (next to compare button) in compare view to see the differences.
https://treeherder.mozilla.org/perfherder/comparechooser
|
|
Assignee | |
Updated•2 years ago
|
|
||
Updated•2 years ago
|
|
|
Assignee | |
Comment 4•2 years ago
|
||
Just wanted to update as I'm still looking into this but haven't quite identified what the issue was. So far I've identified:
- It's the topmost commit https://hg.mozilla.org/integration/autoland/rev/11b26634038d03b2f0ffe4fcf7efa6eec5979847 that causes the inflation of specifically XUL/libXUL (from unpackaging the dmg file)
The most likely files in the commit that could've affected this:
- https://hg.mozilla.org/integration/autoland/diff/11b26634038d03b2f0ffe4fcf7efa6eec5979847/dom/chrome-webidl/moz.build
- https://hg.mozilla.org/integration/autoland/diff/11b26634038d03b2f0ffe4fcf7efa6eec5979847/toolkit/components/moz.build
- https://hg.mozilla.org/integration/autoland/diff/11b26634038d03b2f0ffe4fcf7efa6eec5979847/toolkit/components/uniffi-js/ScaffoldingCall.h
Though I haven't seen any reason why yet since we haven't added any new files in moz.build -- just not building on Android. Reverting those changes and rerunning ./mach package does not seem to change the size of XUL in any case.
It could be likely this is the first time UniFFI is exposed from behind the enable-uniffi-fixtures but don't yet have the data to support that.
|
|
Assignee | |
Comment 5•2 years ago
|
||
Wanted to update on additional diving I've been doing:
I started with packaging Firefox from the commit before the tabs changeset (591a70be8bb271b38380d5ae5ccbeeedd7686e59) with the steps below:
- ./mach package
- Dump the dmg
- Do the same for the top-level tabs commit
- (the prefix of the folder denotes the commit version the dmg was unzipped from)
A 1mb diff in XUL
Next, I then went through each commit in the stack of tabs changes to see what commit caused the increase and determined it was the top-most level commit. This is the comparison of the top two commits in the stack:
5667db8fed85d3c78a9822d22c4d40fe8da93e95 (2rd from the top)
11b26634038d03b2f0ffe4fcf7efa6eec5979847 (top of the stack)
Then I dumped the symbols from XUL and identify any obvious culprits
Running the command: nm -C --print-size --size-sort --radix=d 11b-TabsContents/MacOS/XUL produces a massive file that is too big to post anywhere. Below is a snippet of how the file looks:
i386:x86-64:
0000000000948895 0000000000000001 t ..@1310.branch_instr
0000000123769370 0000000000000001 R .str.114.llvm.10897837265870820446
0000000125159527 0000000000000001 R .str.1386.llvm.1555119068547475091
0000000121850396 0000000000000001 R .str.258.llvm.8409029194902311049
0000000123214863 0000000000000001 R .str.262.llvm.8256954365724220648
0000000123008102 0000000000000001 R .str.31.llvm.15168580881611144265
0000000122115742 0000000000000001 R .str.llvm.5186254772355008316
0000000116302962 0000000000000001 R Pcat1.llvm.13699474806977703579
0000000132031020 0000000000000001 d ipv6Supported
0000000132453752 0000000000000001 b sIncrementalCC
0000000133177224 0000000000000001 b gDoProfileReset
0000000133177152 0000000000000001 b gIsExpectedExit
0000000132443512 0000000000000001 b gEverInitialized
(I had to remove the i386:x86-64, and aarch64 headers in the file to run the python)
I created a python script: https://gist.github.com/skhamis/68ac70be7b99f68bf10c67ae75d1d1c4 that allows me to take two of these files are do the following:
- Take the second, third and fourth column (size, symbol type, and symbol respectively) and turn it into a dictionary
- Identify anything in the second dictionary that is NOT in the first (that means we added)
- Append +NEW to the symbol to easily identified it as a new symbol
- Identify anything in the first dictionary that has been modified more than 10kb
- Append +MOD to the symbol to easily identified as a modified symbol
The result of that python script is here:
https://gist.github.com/skhamis/657678818ca5676ef226b3cc65241648
I’ll also post the top 15 results from that gist (note we filter anything that is not more than 10kb)
Symbol Type Size (in Kb)
anon.f7eed15ed02b7f2abaddf4336a26b240.231.llvm.6736838840929826525+NEW R 249
anon.d722f1ec3ccfe1b6ffdcd0c5574456ed.323.llvm.17613156408877122472+NEW R 247
.str.136.llvm.17781442846136048110+NEW R 195
_ZN12_GLOBAL__N_121gHistogramStringTableE.llvm.5187287869754869448+NEW R 168
_ZN12_GLOBAL__N_121gHistogramStringTableE.llvm.16764379322614431768+NEW R 168
_ZN12_GLOBAL__N_115gHistogramInfosE.llvm.5187287869754869448+NEW R 167
_ZN12_GLOBAL__N_115gHistogramInfosE.llvm.16764379322614431768+NEW R 167
anon.d722f1ec3ccfe1b6ffdcd0c5574456ed.324.llvm.17613156408877122472+NEW D 135
anon.f7eed15ed02b7f2abaddf4336a26b240.232.llvm.6736838840929826525+NEW D 135
.str.llvm.4161228387268301978+NEW R 129
.str.llvm.16719456194283096396+NEW R 121
anon.6f53c457af85820d302824586a27ef90.20.llvm.9334269405363997121+NEW D 107
anon.2db7c95e6d4f28c7428530f5bb1ebc02.20.llvm.12004425837650116441+NEW D 106
.str.31.llvm.11919302484791787419+NEW R 101
.str.31.llvm.17868924908436831308+NEW R 101
If I modify the script to allow any amount of kb and just filter by “tabs”, this is the results:
tabs::sync::engine::TabsSyncImpl::apply_incoming+NEW T 15028
tabs::sync::bridge::TabsBridgedEngine::store_incoming+NEW T 6236
tabs::sync::record::TabsRecord::from_payload+NEW t 5344
tabs::sync::bridge::TabsBridgedEngine::apply+NEW T 3488
<tabs::sync::bridge::BridgedEngineImpl+NEW T 3308
<tabs::sync::engine::TabsEngine+NEW T 1720
tabs::storage::TabsStorage::get_remote_tabs+NEW T 1424
<tabs::FfiConverterTypeTabsError+NEW T 1312
tabs_4d51_TabsBridgedEngine_set_uploaded+NEW T 1296
<tabs::schema::TabsMigrationLogin+NEW T 1064
tabs::sync::engine::TabsSyncImpl::reset+NEW T 912
<tabs::FfiConverterTypeClientRemoteTabs+NEW T 860
tabs_4d51_TabsStore_sync+NEW T 792
tabs::sync::engine::get_registered_sync_engine+NEW T 772
<tabs::FfiConverterTypeRemoteTabRecord+NEW T 604
tabs_4d51_TabsStore_get_all+NEW T 592
tabs_4d51_TabsBridgedEngine_store_incoming+NEW T 576
tabs::storage::TabsStorage::prepare_local_tabs_for_upload+NEW T 548
mozilla::uniffi::ScaffoldingCallHandler<mozilla::uniffi::ScaffoldingObjectConverter<&mozilla::uniffi::kTabsTabsStorePointerType>,+NEW t 488
tabs::sync::engine::<impl+NEW T 484
mozilla::uniffi::ScaffoldingCallHandler<mozilla::uniffi::ScaffoldingObjectConverter<&mozilla::uniffi::kTabsTabsBridgedEnginePointerType>,+NEW t 476
tabs_4d51_TabsBridgedEngine_prepare_for_sync+NEW T 408
tabs::sync::engine::TabsSyncImpl::wipe+NEW T 392
tabs::store::TabsStore::new_with_mem_path+NEW T 388
tabs::sync::<impl+NEW T 380
tabs::storage::TabsStorage::open_or_create+NEW T 380
tabs::storage::TabsStorage::open_if_exists+NEW T 376
tabs::storage::is_url_syncable+NEW t 372
tabs_4d51_TabsStore_reset+NEW T 364
tabs_4d51_TabsStore_set_local_tabs+NEW T 356
tabs::store::TabsStore::set_local_tabs+NEW T 344
<tabs::error::TabsError+NEW T 336
tabs_4d51_TabsBridgedEngine_ensure_current_sync_id+NEW T 328
tabs::store::TabsStore::remote_tabs+NEW T 296
tabs_4d51_TabsBridgedEngine_reset+NEW T 292
tabs_4d51_TabsBridgedEngine_sync_finished+NEW T 292
tabs_4d51_TabsBridgedEngine_wipe+NEW T 292
tabs_4d51_TabsBridgedEngine_set_last_sync+NEW T 292
tabs::sync::bridge::<impl+NEW T 288
tabs_4d51_TabsBridgedEngine_apply+NEW T 284
tabs_4d51_TabsBridgedEngine_sync_id+NEW T 284
tabs_4d51_TabsStore_new+NEW T 280
<tabs::FfiConverterTypeTabsGuid+NEW T 272
core::ptr::drop_in_place<alloc::vec::Vec<tabs::storage::RemoteTab>>+NEW t 264
tabs_4d51_TabsBridgedEngine_reset_sync_id+NEW T 244
<tabs::FfiConverterTypeTabsDeviceType+NEW T 236
mozilla::uniffi::ScaffoldingObjectConverter<&mozilla::uniffi::kTabsTabsStorePointerType>::FromJs(mozilla::dom::OwningDoubleOrArrayBufferOrUniFFIPointer+NEW t 228
mozilla::uniffi::ScaffoldingObjectConverter<&mozilla::uniffi::kTabsTabsBridgedEnginePointerType>::FromJs(mozilla::dom::OwningDoubleOrArrayBufferOrUniFFIPointer+NEW t 228
tabs_4d51_TabsBridgedEngine_last_sync+NEW T 224
tabs::sync::bridge::BridgedEngineImpl::new+NEW T 196
<tabs::storage::RemoteTab+NEW T 188
<tabs::sync::record::TabsRecordTab+NEW T 188
tabs::storage::TabsStorage::new_with_mem_path+NEW T 188
<tabs::sync::record::TabsRecord+NEW T 180
core::ptr::drop_in_place<std::sync::mutex::Mutex<tabs::sync::engine::TabsSyncImpl>>+NEW t 176
<tabs::storage::ClientRemoteTabs+NEW T 172
tabs::sync::engine::TabsEngine::new+NEW T 164
tabs::sync::engine::TabsSyncImpl::new+NEW T 156
tabs::sync::engine::TabsSyncImpl::sync_finished+NEW T 152
tabs::sync::bridge::TabsBridgedEngine::set_uploaded+NEW T 148
mozilla::detail::RunnableFunction<mozilla::uniffi::ScaffoldingCallHandler<mozilla::uniffi::ScaffoldingObjectConverter<&mozilla::uniffi::kTabsTabsStorePointerType>,+NEW t 140
core::ptr::drop_in_place<anyhow::error::ErrorImpl<tabs::error::TabsError>>+NEW t 132
mozilla::detail::RunnableFunction<mozilla::uniffi::ScaffoldingCallHandler<mozilla::uniffi::ScaffoldingObjectConverter<&mozilla::uniffi::kTabsTabsBridgedEnginePointerType>,+NEW t 132
core::ptr::drop_in_place<tabs::error::TabsError>+NEW t 132
tabs::storage::TabsStorage::update_local_state+NEW T 132
core::ptr::drop_in_place<alloc::vec::into_iter::IntoIter<tabs::storage::ClientRemoteTabs>>+NEW t 128
<tabs::sync::engine::STORE_FOR_MANAGER+NEW T 116
tabs::sync::engine::TabsSyncImpl::prepare_for_sync+NEW T 116
core::ptr::drop_in_place<std::sync::poison::PoisonError<std::sync::mutex::MutexGuard<tabs::storage::TabsStorage>>>+NEW t 112
core::ptr::drop_in_place<std::sync::poison::PoisonError<std::sync::mutex::MutexGuard<tabs::sync::engine::TabsSyncImpl>>>+NEW t 112
core::ptr::drop_in_place<std::sync::poison::PoisonError<std::sync::mutex::MutexGuard<alloc::sync::Weak<tabs::store::TabsStore>>>>+NEW t 112
core::ptr::drop_in_place<std::sync::mutex::MutexGuard<alloc::sync::Weak<tabs::store::TabsStore>>>+NEW t 112
core::ptr::drop_in_place<alloc::vec::Vec<tabs::storage::ClientRemoteTabs>>+NEW t 112
core::ptr::drop_in_place<alloc::vec::Vec<tabs::sync::record::TabsRecordTab>>+NEW t 112
core::ptr::drop_in_place<std::sync::mutex::MutexGuard<tabs::storage::TabsStorage>>+NEW t 112
core::ptr::drop_in_place<std::sync::mutex::MutexGuard<tabs::sync::engine::TabsSyncImpl>>+NEW t 112
mozilla::Maybe<mozilla::uniffi::ScaffoldingCallHandler<mozilla::uniffi::ScaffoldingObjectConverter<&mozilla::uniffi::kTabsTabsStorePointerType>,+NEW t 84
mozilla::Maybe<mozilla::uniffi::ScaffoldingCallHandler<mozilla::uniffi::ScaffoldingObjectConverter<&mozilla::uniffi::kTabsTabsBridgedEnginePointerType>,+NEW t 84
tabs::store::TabsStore::get_all+NEW T 84
core::ptr::drop_in_place<core::result::Result<alloc::sync::Arc<tabs::store::TabsStore>,anyhow::Error>>+NEW t 80
core::ptr::drop_in_place<core::result::Result<alloc::sync::Arc<tabs::sync::bridge::TabsBridgedEngine>,anyhow::Error>>+NEW t 80
core::ptr::drop_in_place<tabs::storage::RemoteTab>+NEW t 80
core::ptr::drop_in_place<tabs::sync::record::TabsRecordTab>+NEW t 80
tabs::sync::bridge::TabsBridgedEngine::new+NEW T 80
ffi_tabs_4d51_TabsBridgedEngine_object_free+NEW T 76
ffi_tabs_4d51_TabsStore_object_free+NEW T 76
core::ptr::drop_in_place<tabs::sync::record::TabsRecord>+NEW t 64
core::ptr::drop_in_place<tabs::storage::ClientRemoteTabs>+NEW t 64
_ZL16sTabSharedString.llvm.7379526814677798080+NEW B 64
tabs_4d51_TabsBridgedEngine_sync_started+NEW T 56
tabs_4d51_TabsStore_bridged_engine+NEW T 44
tabs::sync::bridge::TabsBridgedEngine::sync_started+NEW T 40
tabs_4d51_TabsStore_register_with_sync_manager+NEW T 36
<tabs::storage::_::<impl+NEW T 32
<tabs::sync::record::_::<impl+NEW T 32
tabs::storage::devicetype_is_unknown+NEW T 28
mozilla::uniffi::kTabsTabsStorePointerType+NEW d 24
mozilla::uniffi::kTabsTabsBridgedEnginePointerType+NEW d 24
tabs::sync::engine::TabsSyncImpl::get_sync_assoc+NEW T 20
tabs::storage::devicetype_default_deser+NEW T 20
ffi_tabs_4d51_rustbuffer_from_bytes+NEW T 20
core::ptr::drop_in_place<tabs::storage::_::<impl+NEW t 16
core::ptr::drop_in_place<tabs::sync::record::_::<impl+NEW t 16
core::ptr::drop_in_place<std::sync::once::Once::call_once<lazy_static::lazy::Lazy<std::sync::mutex::Mutex<alloc::sync::Weak<tabs::store::TabsStore>>>::get<<tabs::sync::engine::STORE_FOR_MANAGER+NEW t 16
core::ptr::drop_in_place<&tabs::storage::RemoteTab>+NEW t 16
core::ptr::drop_in_place<tabs::sync::engine::TabsEngine>+NEW t 16
core::ptr::drop_in_place<&tabs::sync::record::TabsRecordTab>+NEW t 16
core::ptr::drop_in_place<&alloc::vec::Vec<tabs::storage::RemoteTab>>+NEW t 16
core::ptr::drop_in_place<&alloc::vec::Vec<tabs::sync::record::TabsRecordTab>>+NEW t 16
tabs::uniffi_reexport_hack+NEW T 16
ffi_tabs_4d51_rustbuffer_alloc+NEW T 16
ffi_tabs_4d51_rustbuffer_free+NEW T 16
ffi_tabs_4d51_rustbuffer_reserve+NEW T 16
tabs_uniffi_reexport_hack+NEW T 16
tabs::sync::bridge::TabsBridgedEngine::reset_sync_id+NEW T 16
tabs::sync::bridge::TabsBridgedEngine::set_last_sync+NEW T 16
tabs::sync::bridge::TabsBridgedEngine::sync_finished+NEW T 16
tabs::sync::bridge::TabsBridgedEngine::prepare_for_sync+NEW T 16
tabs::sync::bridge::TabsBridgedEngine::ensure_current_sync_id+NEW T 16
tabs::sync::bridge::TabsBridgedEngine::wipe+NEW T 16
tabs::sync::bridge::TabsBridgedEngine::reset+NEW T 16
tabs::sync::bridge::TabsBridgedEngine::sync_id+NEW T 16
tabs::sync::bridge::TabsBridgedEngine::last_sync+NEW T 16
Which comes to about 11kb, so nowhere near the size the osx-installer instrumented is reporting.
It also seems https://bugzilla.mozilla.org/show_bug.cgi?id=1776386#c6 mentioned that this is potentially expected for instrumented builds? Andrea, is it possible to potentially follow the steps of that bug and consider this an instrumented bloat since tabs symbols are definitely not the culprit here? Thanks!
|
||
Updated•2 years ago
|
|
|
Reporter | |
Comment 6•2 years ago
|
||
hey, Mike, does it make sense to disable the sheriffing on instrumented builds?
|
|
Reporter | |
Updated•2 years ago
|
|
|
||
Comment 8•2 years ago
|
||
Set release status flags based on info from the regressing bug 1791851
|
|
Reporter | |
Updated•2 years ago
|
|
||
Updated•2 years ago
|
Description
•