1306830 - Screenshot --fullpage of large page takes 8 times longer

Status: VERIFIED FIXED

(Core :: Graphics: Text, defect)

Description

[Elbart]

Attachment: screenshot.htm

Bug 1303570 seems to have fixed bug 1268834, but now doing a "screenshot --fullpage" on the attached test-site takes ~44 seconds on my machine, compared to ~5 seconds before.

Last good (time-wise): 2016-09-29
First bad (time-wise): 2016-09-30

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0b9c9000ac93d1e0ddfb603dbeece6ad784754db&tochange=f21ffbf119bd6d7084309123be42f97743f1f7e5

Comment 1

[Alice0775 White]

[Tracking Requested - why for this release]: performance regression.

I can confirm the perf regression on Nightly52.0a1 Windows10.

Comment 2

[Bas Schouten (:bas.schouten)]

Attachment: Bug 1306830: Correctly set the top of the font area to be drawn.

Review commit: https://reviewboard.mozilla.org/r/82680/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/82680/

Comment 3

[Bas Schouten (:bas.schouten)]

I can't seem to reproduce this issue as seriously as reported, however I think the attached patch will do the job, could you confirm Alice?

Comment 4

[Alice0775 White]

(In reply to Bas Schouten (:bas.schouten) from comment #3)
> I can't seem to reproduce this issue as seriously as reported, however I
> think the attached patch will do the job, could you confirm Alice?

The Attachment #8797035 [details] fixed the problem on local build x86 windows10 x64.

Via local build(non pgo, x86):
m-c cset 955840bfd3c2 : it takes about 50 sec
m-c cset 955840bfd3c2  with Attachment #8797035 [details] : It takes about 6 sec

Comment 5

[Marcia Knous [:marcia]]

Tracking 52+ for this regression.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8797035 [details]
Bug 1306830: Correctly set the top of the font area to be drawn.

https://reviewboard.mozilla.org/r/82680/#review82226

Comment 7

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

ping - time for landing? ;)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/98e0a72e854f
Correctly set the top of the font area to be drawn. r=jrmuizel

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/98e0a72e854f

Comment 10

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

[Tracking Requested - why for this release]: Regression

Marking Firefox 51 as affected per uplift to Aurora bug #1303570 comment #56.

Comment 11

[Gerry Chang [:gchang]]

Track 51+ as regression.

Hi :bas.schouten,
could you please nominate this uplift to Beta51?

Comment 12

[Gerry Chang [:gchang]]

Hi Milan,
Can you help find someone to nominate this uplift for Beta51 if bas.schouten is busy working on other bugs?

Comment 13

[Bas Schouten (:bas.schouten)]

Comment on attachment 8797035 [details]
Bug 1306830: Correctly set the top of the font area to be drawn.

Approval Request Comment
[Feature/regressing bug #]: Regressed further by 1303570
[User impact if declined]: Poor performance in some fallback scenarios
[Describe test coverage new/current, TreeHerder]: Aurora
[Risks and why]: Low
[String/UUID change made/needed]: None

Comment 14

[Gerry Chang [:gchang]]

Comment on attachment 8797035 [details]
Bug 1306830: Correctly set the top of the font area to be drawn.

Fix a performance regression. Beta51+. Should be in 51 beta 2.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/edeebc01f04e

Comment 16

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I reproduced this issue using Fx52.0a1, build ID: 0161001030430, on Windows 7 x32.
I can confirm this issue is fixed. I verified using Fx 51.0b2, build ID: 20161121093909 and Fx 52.0a2, build ID: 20161122004020 on Windows 7 x32 and Windows 10 x64.