Closed Bug 1306830 Opened 4 years ago Closed 4 years ago

Screenshot --fullpage of large page takes 8 times longer

Categories

(Core :: Graphics: Text, defect)

52 Branch
x86_64
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla52
Tracking Status
firefox49 --- unaffected
firefox50 --- unaffected
firefox51 + verified
firefox52 + verified

People

(Reporter: elbart, Assigned: bas.schouten)

References

Details

(Keywords: perf, regression)

Attachments

(2 files)

Attached file 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
Blocks: 1303570
[Tracking Requested - why for this release]: performance regression.

I can confirm the perf regression on Nightly52.0a1 Windows10.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: perf, regression
Version: 51 Branch → 52 Branch
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?
Assignee: nobody → bas
Status: NEW → ASSIGNED
Flags: needinfo?(alice0775)
(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
Flags: needinfo?(alice0775)
Tracking 52+ for this regression.
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
Attachment #8797035 - Flags: review?(jmuizelaar) → review+
Flags: needinfo?(bas)
Flags: needinfo?(bas)
Pushed by bschouten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/98e0a72e854f
Correctly set the top of the font area to be drawn. r=jrmuizel
https://hg.mozilla.org/mozilla-central/rev/98e0a72e854f
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
[Tracking Requested - why for this release]: Regression

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

Hi :bas.schouten,
could you please nominate this uplift to Beta51?
Flags: needinfo?(bas)
Hi Milan,
Can you help find someone to nominate this uplift for Beta51 if bas.schouten is busy working on other bugs?
Flags: needinfo?(milan)
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
Flags: needinfo?(milan)
Flags: needinfo?(bas)
Attachment #8797035 - Flags: approval-mozilla-beta?
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.
Attachment #8797035 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.