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)

RESOLVED WONTFIX

Status

()

Core
Printing: Output
RESOLVED WONTFIX
10 months ago
10 months ago

People

(Reporter: igoldan, Unassigned)

Tracking

({regression})

Firefox Tracking Flags

(Not tracked)

Details

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
: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.
Component: Untriaged → Printing: Output
Depends on: 1368948, 1373320
Flags: needinfo?(brsun)
Product: Firefox → Core

Comment 2

10 months ago
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.
Flags: needinfo?(brsun)

Comment 3

10 months ago
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

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

Comment 5

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

Comment 6

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

Comment 7

10 months ago
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.
Status: NEW → RESOLVED
Last Resolved: 10 months ago
Resolution: --- → WONTFIX
Thank you Bruce, for your insightful feedback and for managing this bug.

Updated

10 months ago
Depends on: 1378608
You need to log in before you can comment on or make changes to this bug.