1378050 - 3.13 - 5.14% build times / compiler warnings (windows2012-32, windows2012-64, windows2012-64-noopt, windows8-64) regression on push cd4549c335752aca80ac293015d0b85ada94c332 (Fri Jun 30 2017)

Status: RESOLVED WONTFIX

(Core :: Printing: Output, defect)

Description

[Ionuț Goldan [:igoldan]]

We have detected a build metrics regression from push:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=cd4549c335752aca80ac293015d0b85ada94c332

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

Regressions:

  5%  build times summary windows2012-64 pgo taskcluster-c4.4xlarge     4,543.47 -> 4,776.87
  3%  compiler warnings summary windows8-64 opt                         147.00 -> 152.00
  3%  compiler warnings summary windows2012-64 opt                      147.00 -> 152.00
  3%  compiler warnings summary windows2012-64-noopt debug              153.00 -> 158.00
  3%  compiler warnings summary windows8-64 debug                       153.00 -> 158.00
  3%  compiler warnings summary windows2012-64 debug                    153.00 -> 158.00
  3%  compiler warnings summary windows8-64 pgo                         157.29 -> 162.25
  3%  build times summary windows2012-32 pgo taskcluster-c4.4xlarge     4,265.39 -> 4,399.14
  3%  compiler warnings summary windows2012-64 pgo                      157.08 -> 162.00


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

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]]

:brsun Could you take a look over these issues? Please reply how easy it is to fix regressions above, in hopes we can avoid accepting them.

Comment 2

[Bruce Sun [:brsun]]

Hi Ionuț,

On bug 1368948, we introduce one library, PDFium, into our code base. This library is used to parse PDF files, and to convert them into EMF format. This conversion is a necessary feature for refining our printing feature on Windows platforms. And currently we restrict this library to be built on Windows platforms only.

After replacing the dependent libraries with existing ones (i.e. freetype, libjpeg, zlib), and removing unnecessary files (i.e. openjpeg, xfa), we still need around ~500 source files (*.c, *.cc, *.cpp) and around ~400 header files (*.h, *.hh) as the minimum set of it.

Considering to efforts for maintainability and upgradability, the current solution might be very close to the extreme end that we can afford.

Comment 3

[Mike Hommey [:glandium]]

The build time being affected is pretty much expected. The warnings count, though, shouldn't have increased... it might be worth fixing that (it sounds like this would come from glue code)

Comment 4

[Bruce Sun [:brsun]]

These warnings probably come from PDFium directly because glue codes are not added into m-c yet.

Comment 5

[Mike Hommey [:glandium]]

Ah yes, we don't hide warnings for things that have ALLOW_COMPILER_WARNINGS = True

Comment 6

[Bruce Sun [:brsun]]

Yeah...|ALLOW_COMPILER_WARNINGS = True| is used in our moz.build.

Comment 7

[Bruce Sun [:brsun]]

Extra built time and extra compiler warnings are expected for PDFium library.

Note: This library is built on Nightly only due to the flag |--enable-skia-pdf|. So it won't effect Firefox 56 release.

Comment 8

[Ionuț Goldan [:igoldan]]

Thank you Bruce, for your insightful feedback and for managing this bug.