Skip to content
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.

Sign up for GitHub

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

Reduce ASCII checks in makeInlineImage(). #5187

Merged
merged 1 commit into from
Aug 15, 2014
Merged

Reduce ASCII checks in makeInlineImage(). #5187

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 23 additions & 22 deletions src/core/parser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
FlateStream, isArray, isCmd, isDict, isInt, isName, isNum, isRef,
isString, Jbig2Stream, JpegStream, JpxStream, LZWStream, Name,
NullStream, PredictorStream, Ref, RunLengthStream, warn, info,
StreamType, MissingDataException */
StreamType, MissingDataException, assert */

'use strict';

Expand Down Expand Up @@ -152,32 +152,33 @@ var Parser = (function ParserClosure() {

// searching for the /EI\s/
var state = 0, ch, i, ii;
while (state !== 4 && (ch = stream.getByte()) !== -1) {
switch (ch | 0) {
case 0x20:
case 0x0D:
case 0x0A:
// let's check next five bytes to be ASCII... just be sure
var followingBytes = stream.peekBytes(5);
for (i = 0, ii = followingBytes.length; i < ii; i++) {
var E = 0x45, I = 0x49, SPACE = 0x20, NL = 0xA, CR = 0xD;
while ((ch = stream.getByte()) !== -1) {
if (state === 0) {
state = (ch === E) ? 1 : 0;
} else if (state === 1) {
state = (ch === I) ? 2 : 0;
} else {
assert(state === 2);
if (ch === SPACE || ch === NL || ch === CR) {
// Let's check the next five bytes are ASCII... just be sure.
var n = 5;
var followingBytes = stream.peekBytes(n);
for (i = 0; i < n; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder about this break (and one below) removal, does it improve the performance?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with the break removed it would always scan the 5 chars, wouldnt it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.. but it might open road for some jit optimizations, that's why I'm wondering. Will it be beneficial to add a constant 5 instead of "followingBytes.length" to the for-loop then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add the break back in. I don't know what the JITs will do, but I can put the 5 in its own variable.

ch = followingBytes[i];
if (ch !== 0x0A && ch !== 0x0D && (ch < 0x20 || ch > 0x7F)) {
// not a LF, CR, SPACE or any visible ASCII character
if (ch !== NL && ch !== CR && (ch < SPACE || ch > 0x7F)) {
// Not a LF, CR, SPACE or any visible ASCII character, i.e.
// it's binary stuff. Resetting the state.
state = 0;
break; // some binary stuff found, resetting the state
break;
}
}
state = (state === 3 ? 4 : 0);
break;
case 0x45:
state = 2;
break;
case 0x49:
state = (state === 2 ? 3 : 0);
break;
default:
if (state === 2) {
break; // finished!
}
} else {
state = 0;
break;
}
}
}

Expand Down