New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 897645 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 110002
issue 783855



Sign in to add a comment

clang tot bots failing 'sizes' step

Project Member Reported by h...@chromium.org, Oct 22

Issue description

The step looks like this:

---

# Static initializers in /b/c/b/ToTLinuxASan/src/out/Release/chrome:
# HINT: To get this list, run tools/linux/dump-static-initializers.py
# HINT: diff against the log from the last run to see what changed
# ax_platform_node_auralinux.cc ui::AXPlatformNodeAuraLinux::current_selected_
# ax_platform_node_auralinux.cc base::internal::WeakPtrBase::WeakPtrBase()
# ax_platform_node_auralinux.cc __PRETTY_FUNCTION__._ZN2uiL40AXPlatformNodeAuraLinuxGetStringAtOffsetEP8_AtkTexti18AtkTextGranularityPiS3_+0xa0
# ax_platform_node_auralinux.cc __asan_after_dynamic_init
# ax_platform_node_auralinux.cc __asan_before_dynamic_init
# ax_platform_node_auralinux.cc __dso_handle
# ax_platform_node_auralinux.cc __interceptor___cxa_atexit
# hosted_app_button_container.cc HostedAppButtonContainer::kOriginTotalDuration
# hosted_app_button_container.cc base::time_internal::SaturatedAdd(base::TimeDelta, long)
# hosted_app_button_container.cc __asan_after_dynamic_init
# hosted_app_button_container.cc __asan_before_dynamic_init
# hosted_app_button_container.cc __asan_report_store8
# hosted_app_button_container.cc __func__._ZN24HostedAppButtonContainer22StartTitlebarAnimationEv+0x40
# iostream.cpp std::__1::__start_std_streams
# iostream.cpp std::__1::ios_base::Init::Init()
# iostream.cpp std::__1::ios_base::Init::~Init()
# iostream.cpp typeinfo name for std::__1::__stdoutbuf<wchar_t>+0x19
# iostream.cpp __asan_after_dynamic_init
# iostream.cpp __asan_before_dynamic_init
# iostream.cpp __dso_handle
# iostream.cpp __interceptor___cxa_atexit
# Found 21 static initializers in 3 files.

RESULT chrome: chrome= 1813252088 bytes
RESULT chrome-stripped: stripped= 585564736 bytes
RESULT chrome-text: text= 546292812 bytes
RESULT chrome-data: data= 39263542 bytes
RESULT chrome-bss: bss= 12600768 bytes
RESULT chrome-si: initializers= 24044 files
RESULT chrome-textrel: textrel= 0 relocs

# Static initializers in /b/c/b/ToTLinuxASan/src/out/Release/nacl_helper:
# HINT: To get this list, run tools/linux/dump-static-initializers.py
# HINT: diff against the log from the last run to see what changed
# iostream.cpp std::__1::__start_std_streams
# iostream.cpp std::__1::ios_base::Init::Init()
# iostream.cpp std::__1::ios_base::Init::~Init()
# iostream.cpp typeinfo name for std::__1::__stdoutbuf<wchar_t>+0x19
# iostream.cpp __asan_after_dynamic_init
# iostream.cpp __asan_before_dynamic_init
# iostream.cpp __dso_handle
# iostream.cpp __interceptor___cxa_atexit
# Found 8 static initializers in 1 files.

RESULT nacl_helper: nacl_helper= 335747544 bytes
RESULT nacl_helper-stripped: stripped= 123263272 bytes
RESULT nacl_helper-text: text= 114502903 bytes
RESULT nacl_helper-data: data= 8750278 bytes
RESULT nacl_helper-bss: bss= 11189200 bytes
RESULT nacl_helper-si: initializers= 6374 files
RESULT nacl_helper-textrel: textrel= 0 relocs
RESULT nacl_helper_bootstrap: nacl_helper_bootstrap= 9608 bytes
RESULT nacl_helper_bootstrap-stripped: stripped= 8744 bytes
RESULT nacl_helper_bootstrap-text: text= 6559 bytes
RESULT nacl_helper_bootstrap-data: data= 40 bytes
RESULT nacl_helper_bootstrap-bss: bss= 4113 bytes
RESULT nacl_helper_bootstrap-si: initializers= 0 files
RESULT nacl_helper_bootstrap-textrel: textrel= 0 relocs
RESULT nacl_irt_x86_64.nexe: nacl_irt_x86_64.nexe= 3868208 bytes
RESULT resources.pak: resources.pak= 13291256 bytes
RESULT totals-bss: bss= 23794081 bytes
RESULT totals-data: data= 48013860 bytes
RESULT totals-initializers: initializers= 30418 files
RESULT totals-size: size= 2166168704 bytes
RESULT totals-stripped: stripped= 708836752 bytes
RESULT totals-text: text= 660802274 bytes
RESULT totals-textrel: textrel= 0 relocs
<Thread(Thread-1, started 140524420830976)> ProcessRead: proc.stdout finished.
<Thread(Thread-1, started 140524420830976)> ProcessRead: cleaning up.
<Thread(Thread-2, started daemon 140524412438272)> TimedFlush: Finished
<Thread(Thread-1, started 140524420830976)> ProcessRead: finished.
exit code (as seen by runtest.py): 125
Sending result 2 of 2 to dashboard.
 killed dbus-daemon with PID 21574
 cleared DBUS_SESSION_BUS_ADDRESS environment variable
step returned non-zero exit code: 125
---

I'm not sure what to make of it. Is the upload rejected by the perf dashboard? Something about dbus? Something else?
Cc: thomasanderson@chromium.org
Oct 8 lines up with https://chromium-review.googlesource.com/c/chromium/src/+/1269104 , so it's likely from that.

We don't need to run the static initializer check on *San bots and I wouldn't be surprised if it failed there (even though the list looks somewhat reasonable -- maybe we can make it work eventually? But historically I think we haven't run this step on the *san bots)
Cc: dpranke@chromium.org
thomasanderson is OOO and this is very broken :-(

sizes.py is used to track binary sizes. I think it's unfortunate that it's become conflated with the static initializers check.

It's even more unfortunate that the expectations on number of initializers have become part of this script, and don't take into account the build configuration: does it have ASan? MSan? Is nacl disabled? The script can't tell.

It's probably too late to revert Thomas's change. thakis, dpranke, do you have any ideas here? I don't want to turn off size tracking for these bots; it's nice to have the data.
As far as I know, the sizes step has done the static initializer check for a *very* long time. In fact, for most of that time I thought no one was actually using the sizes part; you Clang folks might be the only people that are using it now.

The current plan of record is to split this script in two, to have one step looking for static initializers and another computing sizes, and then you can run one or both in a given config as desired.

However, this isn't currently a particularly high-priority task for non-clang folks. At some point this was on my plate to do the split, but I'm not sure that I'll get to it soon. I might have some time on Friday or Monday, and I've been trying to clear off a lot of these smaller tasks from my queue, so I might get to it then.

Or, you might want to just take a stab at fixing things or splitting them yourself. 
Owner: h...@chromium.org
Status: Started (was: Available)
> In fact, for most of that time I thought no one was actually using the sizes part; you Clang folks might be the only people that are using it now.

Is there some other mechanism for tracking binary size these days, or are you saying we're the only ones that do it now?

> The current plan of record is to split this script in two, to have one step looking for static initializers and another computing sizes, and then you can run one or both in a given config as desired.

That sounds great. I think it would be cool if all build bots (or at least most) had a sizes step to make it easy to see the binary size impact of a CL for example.

> Or, you might want to just take a stab at fixing things or splitting them yourself.

I'll probably just try to disable it on the bots where it's failing. It's not high priority for us either.

https://chromium-review.googlesource.com/c/chromium/tools/build/+/1322809
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/b09175a1614602ed730a85d5cdbce66c964e1398

commit b09175a1614602ed730a85d5cdbce66c964e1398
Author: Hans Wennborg <hans@chromium.org>
Date: Wed Nov 07 12:38:13 2018

Disable the sizes step on Clang ToT bots where it doesn't work anymore

Recently the sizes step also started enforcing the expected number of static
initializers. This doesn't work for more exotic build configs such as ASan, see
bug.

Bug:  897645 
Change-Id: If7f303a054b41ab402f0ac891b57de1485ba9bbb
Reviewed-on: https://chromium-review.googlesource.com/c/1322809
Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Hans Wennborg <hans@chromium.org>

[modify] https://crrev.com/b09175a1614602ed730a85d5cdbce66c964e1398/scripts/slave/recipe_modules/chromium_tests/chromium_clang.py

Cc: agrieve@chromium.org
> Is there some other mechanism for tracking binary size these days, 
> or are you saying we're the only ones that do it now?

I'm not sure where all or how this is tracked. I know Android tracks binary size closely, but I think they have a different mechanism for this. 

+agrieve who can probably answer that.

I'm not sure if any other platform is regularly tracking sizes, though.
Blockedon: 783855
Blocking: -894363
Labels: -Pri-1 Pri-2
Owner: thomasanderson@chromium.org
agrieve: What about Windows, Mac and Linux?


Anyway, the bots that were red due to this are now green, so unblocking the roll and reassigning to thomasanderson.
Cc: alexclarke@chromium.org
Windows tracks "sizes" I believe, and there's a sheriffing rotation that covers these regressions.

Not sure if Mac and Linux are covered by this too, maybe the current sheriff can elaborate. 


Blockedon: 110002
Status: Assigned (was: Started)
 Bug 110002  is tracking removing the SI check from sizes.py.  Once that's done, we can reenable the sizes step on the clang ToT bots in case anyone is tracking binary sizes on those bots using the perf dashboard.
Project Member

Comment 13 by bugdroid1@chromium.org, Dec 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/ff9378a845b7520620d16109e6444dbfebd3a2fa

commit ff9378a845b7520620d16109e6444dbfebd3a2fa
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Wed Dec 05 18:36:24 2018

Enable the sizes step on Clang ToT bots where it was disabled

The sizes step should succeed now since the static initializer check
has been moved to a separate testing script.

BUG= 897645 

Change-Id: Ic50acbb5d99d1ba3ca9d817b935325c047e26190
Reviewed-on: https://chromium-review.googlesource.com/c/1361806
Reviewed-by: Hans Wennborg <hans@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>

[modify] https://crrev.com/ff9378a845b7520620d16109e6444dbfebd3a2fa/scripts/slave/recipe_modules/chromium_tests/chromium_clang.py

Status: Fixed (was: Assigned)

Sign in to add a comment