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
Faster checkReservedWord #13386
Faster checkReservedWord #13386
Changes from all commits
c6d17ff
e2ded8a
dcbfd45
6c4fc16
84be70f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import Benchmark from "benchmark"; | ||
import baseline from "../../lib/index-main.js"; | ||
import current from "../../lib/index.js"; | ||
import { report } from "../util.mjs"; | ||
|
||
const suite = new Benchmark.Suite(); | ||
function createInput(length) { | ||
return "abcde12345z;".repeat(length); | ||
} | ||
current.parse("a"); | ||
function benchCases(name, implementation, options) { | ||
for (const length of [64, 128, 256, 512]) { | ||
const input = createInput(length); | ||
suite.add(`${name} ${length} length-11 identifiers`, () => { | ||
implementation.parse(input, options); | ||
}); | ||
} | ||
} | ||
|
||
benchCases("baseline", baseline); | ||
benchCases("current", current); | ||
|
||
suite.on("cycle", report).run(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import Benchmark from "benchmark"; | ||
import baseline from "../../lib/index-main.js"; | ||
import current from "../../lib/index.js"; | ||
import { report } from "../util.mjs"; | ||
|
||
const suite = new Benchmark.Suite(); | ||
function createInput(length) { | ||
return "await;".repeat(length); | ||
} | ||
current.parse("a"); | ||
function benchCases(name, implementation, options) { | ||
for (const length of [64, 128, 256, 512]) { | ||
const input = createInput(length); | ||
suite.add(`${name} ${length} await identifier`, () => { | ||
implementation.parse(input, options); | ||
}); | ||
} | ||
} | ||
|
||
benchCases("baseline", baseline); | ||
benchCases("current", current); | ||
|
||
suite.on("cycle", report).run(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,3 +21,66 @@ export const keywordRelationalOperator = /^in(stanceof)?$/; | |
export function isIteratorStart(current: number, next: number): boolean { | ||
return current === charCodes.atSign && next === charCodes.atSign; | ||
} | ||
|
||
// This is the comprehensive set of JavaScript reserved words | ||
// If a word is in this set, it could be a reserved word, | ||
// depending on sourceType/strictMode/binding info. In other words | ||
// if a word is not in this set, it is not a reserved word under | ||
// any circumstance. | ||
const reservedWordLikeSet = new Set([ | ||
"break", | ||
"case", | ||
"catch", | ||
"continue", | ||
"debugger", | ||
"default", | ||
"do", | ||
"else", | ||
"finally", | ||
"for", | ||
"function", | ||
"if", | ||
"return", | ||
"switch", | ||
"throw", | ||
"try", | ||
"var", | ||
"const", | ||
"while", | ||
"with", | ||
"new", | ||
"this", | ||
"super", | ||
"class", | ||
"extends", | ||
"export", | ||
"import", | ||
"null", | ||
"true", | ||
"false", | ||
"in", | ||
"instanceof", | ||
"typeof", | ||
"void", | ||
"delete", | ||
// strict | ||
"implements", | ||
"interface", | ||
"let", | ||
"package", | ||
"private", | ||
"protected", | ||
"public", | ||
"static", | ||
"yield", | ||
// strictBind | ||
"eval", | ||
"arguments", | ||
// reservedWorkLike | ||
"enum", | ||
"await", | ||
]); | ||
|
||
export function canBeReservedWord(word: string): boolean { | ||
return reservedWordLikeSet.has(word); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To avoid maintaining another reserved words list, could this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of that long list, why not do something simple as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The The
I agree it is not ideal to duplicate keywords, WDYT if I add
@KFlash Can you elaborate? How can There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 for deferring it to 7.15 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @JLHwung It's possible if you make things complex and add some bitwise masks behind each reserved word token. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you verify whether using a prototye-less object is faster? My intuition is that Sets are algorithmically fast, but the actual implementation in V8 is slow. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jridgewell It's faster because I "pack" all reserved words into a set of bitwise masks. It also consumes less memory. You are only dealing with series of number. see Kataw for an example. Same you can do with tokens, keywords, and everything you need. No need for a lookup. Witch one is fastest? Set and lookup or this one or a switch statement with 60 cases and 100lines of code to get all expression nodes or this one?
It's very obvious what's fastest, but I think it's out of scope for Babel There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jridgewell I come up with a benchmark file comparing these three approaches: Set#has, Object brand checks and (switch length + string equals). The benchmark result on Node.js 16.1.0:
On different string cases, where each input is in different memory address, On different string cases, @KFlash Yes we can do a single bitwise check if we replace the data structure of token storage and create different token for different keywords. However Babel currently exposes its token storage via |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or maybe we can move this check after the
if yield/await/arguments
checks, and just useisStrictBindReservedWord
? Intuitively it shouldn't be slower, but I didn't check.