-
Notifications
You must be signed in to change notification settings - Fork 10.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize a font-heavy document #5071
Conversation
This patch replaces some vanilla arrays with typed arrays, and avoids some array copying. It reduces the peak RSS when viewing http://www.dynacw.co.jp/Portals/3/fontsamplepdf/sample_4942546800828.pdf from ~940 MiB to ~750 MiB, and reduces its load time from 83 to 76 ms.
This avoids the creation of over two million array objects when viewing http://www.dynacw.co.jp/Portals/3/fontsamplepdf/sample_4942546800828.pdf, and reduces load time from 76 to 73 ms.
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/7d97c0e44bbb81a/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/8afb31b9dfb845d/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/7d97c0e44bbb81a/output.txt Total script time: 21.12 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/8afb31b9dfb845d/output.txt Total script time: 23.06 mins
|
/botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/e454b441c37a666/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/e454b441c37a666/output.txt Total script time: 0.92 mins Published
|
The preview is slower for me (i.e. more than a second rendering time per page extra) than the current master. It it just me or can anyone else also reproduce this? |
I just redid the profiling with Firefox, and I got a 13% improvement in "Overall" time, going from 85ms to 74ms. |
Hm, I guess it's just me then, because I just tried again with a cleared cache and a browser restart and it still takes longer with your patch. Strange... |
Tim, what document are you testing with? I can try to verify your results. |
I'm using the document mentioned in the PR description, i.e. http://www.dynacw.co.jp/Portals/3/fontsamplepdf/sample_4942546800828.pdf. I'm using Nightly 34 on Windows 7. The demo viewer is much faster than the viewer from this PR somehow. I have downloaded the file to make sure network lags are no problem (and opened the file from the local harddisk). |
the benchmark is checking what? because when i scroll through the pages they all require noticeable amounts of time. Ill get some numbers... |
Ok scrolling down to page 10 gives me these numbers from pdfbug=stats. MacOs Chrome 36:
With the PR merged I get these numbers:
looks for me like a good 5% speedup |
I did the same thing now. Scrolling down from page 1 to page 10 with pdfBug=Stats enabled. With the current master, I get: Page Request: 2130ms With the generated preview above I get: Page Request: 12343ms A massive difference. But if both of you cannot replicate those numbers, then it must be something else. Not sure what though: the only difference is the used viewer. The file is loaded from harddisk in both cases and the configuration is exactly the same. I also see nothing in the patch that could cause such a delay. |
to be precise: I took master and master+pr. With preview I get:
Will ask a windows colleague to check. |
I'll also try master and master + PR too. Maybe something is wrong with the preview. |
With master (locally) I get: Page Request 676ms With master + PR I get: Page Request 352ms All of a sudden a very large improvement (also visually), whereas the preview was horribly slow. I'm flabbergasted, but since your numbers also confirm this measurement, it must be something with the preview in combination with my configuration. If I run the test multiple times, the overall time is even lower on average. |
There seems to be some windows issues with stability tests. On a Chrome 35 on WIndows 7 I got these numbers: master
preview
And preview was also visually slower |
Weird stuff, but at least the PR is not the culprit. Sorry for the noise :) |
Ready to merge? |
@yurydelendik: anything else I need to do here? Thanks. |
Thank you for the patch. We still perform bytesToString/stringToArray dance. I think it will be better to do it it three steps: 1) count amount of table entries, padded table sizes, total size of the headers, and allocate single typed array based on that, 2) write all table entries (without creating of temp arrays), 3) write all table data. Also Uint16Array might be changed to Uint8Array. |
Optimize a font-heavy document
That sounds promising. I'll continue profiling and see if this part comes up as still significant. |
I've done some optimizations that reduce the time and space required to view http://www.dynacw.co.jp/Portals/3/fontsamplepdf/sample_4942546800828.pdf.