Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(379)

Issue 2430313006: Revert of Traverse PDF page tree only once in CPDF_Document (Closed)

Created:
4 years, 2 months ago by dsinclair
Modified:
4 years, 2 months ago
Reviewers:
Lei Zhang, Tom Sepez, npm
CC:
pdfium-reviews_googlegroups.com
Target Ref:
refs/heads/master
Project:
pdfium
Visibility:
Public.

Description

Revert of Traverse PDF page tree only once in CPDF_Document (patchset #4 id:60001 of https://codereview.chromium.org/2414423002/ ) Reason for revert: Possible cause of crbug.com/657897 reverting to find out. BUG=657897 Original issue's description: > Traverse PDF page tree only once in CPDF_Document > > In our current implementation of CPDF_Document::GetPage, we traverse > the PDF page tree until we find the index we are looking for. This is > slow when we do calls GetPage(0), GetPage(1), ... since in this case > the page tree will be traversed n times if there are n pages. This CL > makes sure the page tree is only traversed once. > > Time to load the PDF from the bug below in chrome official build: > Before this CL: 1 minute 40 seconds > After this CL: 5 seconds > > BUG=chromium:638513 > > Committed: https://pdfium.googlesource.com/pdfium/+/7c29e27dae139a205755c1a29b7f3ac8b36ec0da TBR=thestig@chromium.org,tsepez@chromium.org,npm@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=chromium:638513 Committed: https://pdfium.googlesource.com/pdfium/+/7403f8a2a0d87179a1ccd57ceccd2d84fa59c319

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+118 lines, -150 lines) Patch
M core/fpdfapi/parser/cpdf_document.h View 3 chunks +5 lines, -12 lines 0 comments Download
M core/fpdfapi/parser/cpdf_document.cpp View 7 chunks +113 lines, -138 lines 0 comments Download

Messages

Total messages: 10 (4 generated)
dsinclair
Created Revert of Traverse PDF page tree only once in CPDF_Document
4 years, 2 months ago (2016-10-20 17:23:33 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2430313006/1
4 years, 2 months ago (2016-10-20 17:23:37 UTC) #3
Tom Sepez
stampy stamp LGTM
4 years, 2 months ago (2016-10-20 17:41:22 UTC) #4
commit-bot: I haz the power
Try jobs failed on following builders: android on master.tryserver.client.pdfium (JOB_FAILED, https://build.chromium.org/p/tryserver.client.pdfium/builders/android/builds/1209)
4 years, 2 months ago (2016-10-20 17:50:27 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2430313006/1
4 years, 2 months ago (2016-10-20 17:50:57 UTC) #8
commit-bot: I haz the power
4 years, 2 months ago (2016-10-20 17:58:55 UTC) #10
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://pdfium.googlesource.com/pdfium/+/7403f8a2a0d87179a1ccd57ceccd2d84fa59...

Powered by Google App Engine
This is Rietveld 408576698