1473465 - 2.16 - 3.65% compiler_metrics num_static_constructors (windows2012-32) regression on push 3d7f2fdc5bf7ca7521013b28e0d8be0785d2b58a (Wed Jul 4 2018)

Status: RESOLVED WONTFIX

(Core :: Graphics: CanvasWebGL, defect, P3)

Description

[Ionuț Goldan [:igoldan]]

We have detected a build metrics regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=3d7f2fdc5bf7ca7521013b28e0d8be0785d2b58a

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

  4%  compiler_metrics num_static_constructors windows2012-32 opt      514.00 -> 532.75
  2%  compiler_metrics num_static_constructors windows2012-32 debug    870.00 -> 888.75


You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=14180

On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Automated_Performance_Testing_and_Sheriffing/Build_Metrics

Comment 1

[Kelsey Gilbert [:jgilbert]]

Yep, that's ANGLE updates for you!

Comment 2

[Kelsey Gilbert [:jgilbert]]

Oh! This was the WebGPU bindings, hmm...

Comment 3

[Kelsey Gilbert [:jgilbert]]

So this is +18 static ctors in both cases. I didn't deliberately add static ctors.

In WebIDL, I do add 19 concrete interfaces, but that's the closest to 18 I can find.
Regardless, I suspect this is a result of generated bindings classes.
:qdot, is that likely?

Also, is there a place where we get a dump of the static ctor names? That would make this way easier! (and if we count them, we should be able to list them!)

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

:froydnj- can you answer :jgilberts question about getting a dump of the static ctor names?

Comment 5

[Nathan Froyd [:froydnj]]

(In reply to Joel Maher ( :jmaher ) (UTC+2) (PTO: Back July 17) from comment #4)
> :froydnj- can you answer :jgilberts question about getting a dump of the
> static ctor names?

You can look at the breakpad .sym files (that's from the target.crashreporter-symbols.zip file that you can get from clicking on a "B" job in treeherder and looking at the "Job" tab) and search for "`dynamic initializer for'", as our tests do:

https://searchfox.org/mozilla-central/source/toolkit/crashreporter/tools/symbolstore.py#593-596

so grepping .sym files and sorting, for before and after, then diffing the results should be illuminating.

Comment 6

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Clearing ni? for now since it looks like there's more work to do on isolating the static ctors generated.

Comment 7

[Ionuț Goldan [:igoldan]]

:jgilbert have you filed any bugs related to these compiler warnings? If so, please link them with this bug.

Comment 8

[Ionuț Goldan [:igoldan]]

Clearing ni? as :nfroyd provided the answer.

Comment 9

[Kelsey Gilbert [:jgilbert]]

$ diff pre.txt post.txt
3304a3305,3323
> static void mozilla::dom::WebGPUBindingType_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUBlendFactor_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUBlendOperation_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUBufferUsage_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUColorWriteBits_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUCompareFunction_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUFilterMode_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUIndexFormat_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUInputStepMode_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPULoadOp_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUPrimitiveTopology_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUShaderStageBit_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUShaderStage_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUStencilOperation_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUStoreOp_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUTextureDimension_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUTextureFormat_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUTextureUsage_Binding::`dynamic initializer for 'sConstants_specs''()
> static void mozilla::dom::WebGPUVertexFormat_Binding::`dynamic initializer for 'sConstants_specs''()

https://dxr.mozilla.org/mozilla-central/source/obj-x86_64-pc-linux-gnu/dom/bindings/WebGPUBinding.cpp#8752

> namespace WebGPUBindingType_Binding {
> [...]
> static const ConstantSpec sConstants_specs[] = {
>   { "UNIFORM_BUFFER", JS::NumberValue(0U) },
>   { "SAMPLER", JS::NumberValue(1U) },
>   { "SAMPLED_TEXTURE", JS::NumberValue(2U) },
>   { "STORAGE_BUFFER", JS::NumberValue(3U) },
>   { 0, JS::UndefinedValue() }
> };

I don't think there's much to be done about these. (fyi-ni :qdot)

Comment 10

[Kelsey Gilbert [:jgilbert]]

FWIW the process here was:
- Download a Before and After target.crashreporter-symbols.zip from the B job
- Extract
- grep -Ehor 'static.+dynamic initializer for.+$' Before/xul.pdb|sort|uniq > Before.txt
 - (E: extended ('full') regex, h: omit filename, o: only matched text, r: recursive)
- (same with After)
- diff Before.txt After.txt

Comment 11

[Nathan Froyd [:froydnj]]

Nice, thank you for looking.

We liberally applied constexpr and other tricks to Value.h to make DOM bindings static constructors go away at one point; I guess it's time to do it with feeling.

Comment 12

[Kelsey Gilbert [:jgilbert]]

FWIW, there's a very good chance most (all?) of these constants will become WebIDL enums in the near future, too.

Comment 13

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Yeah, this should be fine for the moment. We can file a followup on the constexpr changes.

Comment 14

[Nathan Froyd [:froydnj]]

(In reply to Kyle Machulis [:qdot] [:kmachulis] (if a patch has no decent commit message, automatic r-) from comment #13)
> Yeah, this should be fine for the moment. We can file a followup on the
> constexpr changes.

Filed bug 1478671 on the constexpr changes.