1433910 - 5.9 - 7.57% build times (windows2012-32, windows2012-64) regression on push 75b5af7910068a6fe04c61b267d985c80362e49d (Fri Jan 26 2018)

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Ionuț Goldan [:igoldan]]

We have detected a build metrics regression from push:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=415be6173ceacf2c89cf9d232f9988bac7d74bfb&tochange=75b5af7910068a6fe04c61b267d985c80362e49d

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

Regressions:

  8%  build times windows2012-32 opt static-analysis taskcluster-c4.4xlarge     2,129.09 -> 2,290.32
  6%  build times windows2012-64 opt static-analysis taskcluster-c4.4xlarge     2,255.87 -> 2,388.99


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

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

[Ionuț Goldan [:igoldan]]

:kvark bug 1433139 increased our build times on Windows platforms. Was this regression expected? Can we do something to reduce it or should we accept it?

Comment 2

[Dzmitry Malyshau [:kvark]]

Ionut, the changes add one extra (fairly lightweight) dependency to https://github.com/ron-rs/ron, so some increase in build times is expected. I'll take a closer look at those to see if we can do anything.

Comment 3

[Dzmitry Malyshau [:kvark]]

Confirmed this due to serialization derives that are required for capturing (local WR build time jumps from 150 to 320 sec in release).
Filed https://github.com/serde-rs/serde/issues/1153 upstream to see if this can be optimized.

Another idea is to split the serialization feature from deserialization, since only the former is needed for end-users of Nightly to provide us with captures.

Comment 4

[Nathan Froyd [:froydnj]]

(In reply to Dzmitry Malyshau [:kvark] from comment #3)
> Another idea is to split the serialization feature from deserialization,
> since only the former is needed for end-users of Nightly to provide us with
> captures.

This would be desirable: it looks like bug 1433139 added ~400K of code to libxul, which is absolutely massive:

https://treeherder.mozilla.org/perf.html#/graphs?series=mozilla-inbound,1299710,1,2&zoom=1516953879055.2825,1517011541417.6904,62597014.92537314,63343283.58208955&selected=mozilla-inbound,1299710,299934,402685860,2

Any reduction to that amount of code would be good.

Comment 5

[Bobby Holley (:bholley)]

(In reply to Nathan Froyd [:froydnj] from comment #4)
> (In reply to Dzmitry Malyshau [:kvark] from comment #3)
> > Another idea is to split the serialization feature from deserialization,
> > since only the former is needed for end-users of Nightly to provide us with
> > captures.
> 
> This would be desirable: it looks like bug 1433139 added ~400K of code to
> libxul, which is absolutely massive:

This understates the problem. It added 400k to the compressed installer, which means more like 800k to libxul.

Our experience suggests that it's easy to accidentally trigger tons of extra monomorphization with Rust code, and that this increase in codesize is highly correlated with build times.

A good starting point is to see if you've added any new large functions in [1]. I've recently done some codesize analysis, so feel free to ping me if you need any with the analysis.

[1] nm --print-size --size-sort --radix=d libxul.so

Comment 6

[Nathan Froyd [:froydnj]]

(In reply to Bobby Holley (:bholley) from comment #5)
> A good starting point is to see if you've added any new large functions in
> [1]. I've recently done some codesize analysis, so feel free to ping me if
> you need any with the analysis.

These functions look like they're from bug 1433139.  The top three are among the largest entities in libxul (functions and objects) and the top seven are in the top 100:

76568 FUNC    LOCAL  DEFAULT   14 _ZN300_$LT$webrender_api..display_item.._IMPL_DESERIALIZE_FOR_CompletelySpecificDisplayItem..$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender_api..display_item..CompletelySpecificDisplayItem$GT$..deserialize..__Visitor$LT$$u27$de$GT$$u20$as$u20$serde..de..Visitor$LT$$u27$de$GT$$GT$10visit_enum17h707846458c9ca607E
32052 FUNC    LOCAL  DEFAULT   14 _ZN264_$LT$webrender..resource_cache.._IMPL_DESERIALIZE_FOR_PlainCacheOwn..$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender..resource_cache..PlainCacheOwn$GT$..deserialize..__Visitor$LT$$u27$de$GT$$u20$as$u20$serde..de..Visitor$LT$$u27$de$GT$$GT$9visit_map17h62035e6c59f3758aE
27582 FUNC    LOCAL  DEFAULT   14 _ZN260_$LT$webrender..render_task.._IMPL_DESERIALIZE_FOR_RenderTaskKind..$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender..render_task..RenderTaskKind$GT$..deserialize..__Visitor$LT$$u27$de$GT$$u20$as$u20$serde..de..Visitor$LT$$u27$de$GT$$GT$10visit_enum17hc0c54e28acdd3959E
26141 FUNC    LOCAL  DEFAULT   14 _ZN232_$LT$webrender..tiling.._IMPL_DESERIALIZE_FOR_Frame..$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender..tiling..Frame$GT$..deserialize..__Visitor$LT$$u27$de$GT$$u20$as$u20$serde..de..Visitor$LT$$u27$de$GT$$GT$9visit_map17hbaa7c5a291307174E
24724 FUNC    LOCAL  DEFAULT   14 _ZN244_$LT$webrender..batch.._IMPL_DESERIALIZE_FOR_AlphaBatcher..$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender..batch..AlphaBatcher$GT$..deserialize..__Visitor$LT$$u27$de$GT$$u20$as$u20$serde..de..Visitor$LT$$u27$de$GT$$GT$9visit_map17h5c2dd9eab10cc09cE
21921 FUNC    LOCAL  DEFAULT   14 _ZN260_$LT$webrender..render_task.._IMPL_DESERIALIZE_FOR_RenderTaskTree..$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender..render_task..RenderTaskTree$GT$..deserialize..__Visitor$LT$$u27$de$GT$$u20$as$u20$serde..de..Visitor$LT$$u27$de$GT$$GT$9visit_map17hd392351f558b3a3cE
18969 FUNC    LOCAL  DEFAULT   14 _ZN242_$LT$webrender..gpu_cache.._IMPL_DESERIALIZE_FOR_Texture..$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender..gpu_cache..Texture$GT$..deserialize..__Visitor$LT$$u27$de$GT$$u20$as$u20$serde..de..Visitor$LT$$u27$de$GT$$GT$9visit_map17hb1dcf225c10279feE
17808 FUNC    LOCAL  DEFAULT   14 _ZN272_$LT$webrender..tiling.._IMPL_DESERIALIZE_FOR_RenderTargetList..$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender..tiling..RenderTargetList$LT$T$GT$$GT$..deserialize..__Visitor$LT$$u27$de$C$$u20$T$GT$$u20$as$u20$serde..de..Visitor$LT$$u27$de$GT$$GT$9visit_map17he19bbad9e33b9d5bE
16796 FUNC    LOCAL  DEFAULT   14 _ZN250_$LT$webrender..tiling.._IMPL_DESERIALIZE_FOR_RenderPassKind..$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender..tiling..RenderPassKind$GT$..deserialize..__Visitor$LT$$u27$de$GT$$u20$as$u20$serde..de..Visitor$LT$$u27$de$GT$$GT$10visit_enum17hb02f9f695d6deb71E
16508 FUNC    LOCAL  DEFAULT   14 _ZN268_$LT$webrender_api..display_item.._IMPL_DESERIALIZE_FOR_BorderDetails..$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender_api..display_item..BorderDetails$GT$..deserialize..__Visitor$LT$$u27$de$GT$$u20$as$u20$serde..de..Visitor$LT$$u27$de$GT$$GT$10visit_enum17h0605884a04ade5ddE
14326 FUNC    LOCAL  DEFAULT   14 _ZN266_$LT$webrender..resource_cache.._IMPL_DESERIALIZE_FOR_PlainResources..$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender..resource_cache..PlainResources$GT$..deserialize..__Visitor$LT$$u27$de$GT$$u20$as$u20$serde..de..Visitor$LT$$u27$de$GT$$GT$9visit_map17h5cf916516e8d5af0E
14009 FUNC    LOCAL  DEFAULT   14 _ZN266_$LT$webrender..glyph_rasterizer.._IMPL_DESERIALIZE_FOR_FontInstance..$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender..glyph_rasterizer..FontInstance$GT$..deserialize..__Visitor$LT$$u27$de$GT$$u20$as$u20$serde..de..Visitor$LT$$u27$de$GT$$GT$9visit_map17h3d49172f1729cf45E
11199 FUNC    LOCAL  DEFAULT   14 _ZN307_$LT$webrender_api..display_item.._IMPL_DESERIALIZE_FOR_SpecificDisplayItem..$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender_api..display_item..SpecificDisplayItem$GT$..deserialize_in_place..__Visitor$LT$$u27$de$C$$u20$$u27$place$GT$$u20$as$u20$serde..de..Visitor$LT$$u27$de$GT$$GT$10visit_enum17h4d0ce41e3b3ac465E
 9368 FUNC    LOCAL  DEFAULT   14 _ZN9webrender6tiling38_IMPL_DESERIALIZE_FOR_TextureAllocator103_$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender..tiling..TextureAllocator$GT$11deserialize17h8d06cb7c8a8bcee5E
 3612 FUNC    LOCAL  DEFAULT   14 _ZN13webrender_api5image39_IMPL_DESERIALIZE_FOR_ExternalImageData107_$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender_api..image..ExternalImageData$GT$11deserialize17h019f7b1dee964d66E
 2920 FUNC    LOCAL  DEFAULT   14 _ZN13webrender_api12display_item28_IMPL_DESERIALIZE_FOR_ClipId103_$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender_api..display_item..ClipId$GT$11deserialize17h4c74a50ba97e02e9E
 2478 FUNC    LOCAL  DEFAULT   14 _ZN13webrender_api5color28_IMPL_DESERIALIZE_FOR_ColorF96_$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender_api..color..ColorF$GT$11deserialize17hf0b474bfd0b3c1a0E
 1276 FUNC    LOCAL  DEFAULT   14 _ZN13webrender_api3api32_IMPL_DESERIALIZE_FOR_PipelineId98_$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender_api..api..PipelineId$GT$11deserialize17hbf005b40441fd1e7E
 1072 FUNC    LOCAL  DEFAULT   14 _ZN9webrender9gpu_cache32_IMPL_DESERIALIZE_FOR_BlockIndex100_$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender..gpu_cache..BlockIndex$GT$11deserialize17h94fcc182ef90c152E
  837 FUNC    LOCAL  DEFAULT   14 _ZN13webrender_api12display_item33_IMPL_DESERIALIZE_FOR_ClipChainId108_$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender_api..display_item..ClipChainId$GT$11deserialize17hea703efae91f1eb8E
  591 FUNC    LOCAL  DEFAULT   14 _ZN9webrender14internal_types37_IMPL_DESERIALIZE_FOR_RenderPassIndex110_$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender..internal_types..RenderPassIndex$GT$11deserialize17ha841851101578a5dE
  591 FUNC    LOCAL  DEFAULT   14 _ZN9webrender14internal_types36_IMPL_DESERIALIZE_FOR_CacheTextureId109_$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender..internal_types..CacheTextureId$GT$11deserialize17h7668259bd414fa6dE
  546 FUNC    LOCAL  DEFAULT   14 _ZN13webrender_api12display_item28_IMPL_DESERIALIZE_FOR_ClipId103_$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$webrender_api..display_item..ClipId$GT$11deserialize17haf189a297b8f4d28E
  367 FUNC    LOCAL  DEFAULT   14 _ZN8audioipc8messages34_IMPL_DESERIALIZE_FOR_StreamParams100_$LT$impl$u20$serde..de..Deserialize$LT$$u27$de$GT$$u20$for$u20$audioipc..messages..StreamParams$GT$11deserialize17he1afa5ef47541303E

But that's only about 350k of code.  So either:

1) There's a lot of other smaller functions; or
2) The 800k figure is way off; or
3) I am looking at the wrong things (entirely possible!).

Comment 7

[Dzmitry Malyshau [:kvark]]

Interesting, thanks for the numbers! For the record, the first entry is not getting enabled by capturing infrastructure, it's always enabled for message serialization across IPC boundary (talking about "_IMPL_DESERIALIZE_FOR_CompletelySpecificDisplayItem").

Comment 8

[Dzmitry Malyshau [:kvark]]

Also, the fix is coming in WR: https://github.com/servo/webrender/pull/2359

Comment 9

[Nathan Froyd [:froydnj]]

(In reply to Dzmitry Malyshau [:kvark] from comment #7)
> Interesting, thanks for the numbers! For the record, the first entry is not
> getting enabled by capturing infrastructure, it's always enabled for message
> serialization across IPC boundary (talking about
> "_IMPL_DESERIALIZE_FOR_CompletelySpecificDisplayItem").

Ah, thanks for the clarification!  So all of the _IMPL_DESERIALIZE_FOR functions are for IPC messaging?  If that's true, we have 350k of codesize for IPC serialization just for webrender types?  That seems...excessive.

Comment 10

[Dzmitry Malyshau [:kvark]]

Nathan,
Actually, no, I was wrong about this first one: most of the `_IMPL_DESERIALIZE_FOR` are for new stuff.
Looks like the fix is landing now - https://bugzilla.mozilla.org/show_bug.cgi?id=1433567#c7

Comment 11

[Dzmitry Malyshau [:kvark]]

Graphs show it to be fixed as of rev 0c95a19decb5 (https://hg.mozilla.org/integration/mozilla-inbound/rev/0c95a19decb5) where my WebRender changes landed to keep only serialization derived for Gecko builds:
https://treeherder.mozilla.org/perf.html#/graphs?timerange=172800&series=mozilla-inbound,1531360,1,2&selected=mozilla-inbound,1531360,301129,404912456,2