This PR adds benchmarks for the lexer. I'm doing some work on optimizing
the lexer and I thought it'd be useful to see the effects of changes in
isolation, separate from the parser.
These benchmarks may not be ideal to keep long-term, but for now it'd be
useful.
In order to do so, it's necessary for `oxc_parser` crate to expose the
lexer, but have done that without adding it to the docs, and using an
alias `__lexer`.
Small optimization to the lexer.
Whitespace, line breaks, and comments are all skipped by
`read_next_token()`.
At present there's a different `Kind` for each, and `read_next_token()`
decides whether to skip with `matches!(kind, Kind::WhiteSpace |
Kind::NewLine | Kind::Comment | Kind::MultiLineComment)`.
These `Kind`s are used for no other purpose, so there seems little
reason to differentiate them.
This PR combines them all into `Kind::Skip`, so then the test of whether
to skip is reduced to `kind == Kind::Skip`.
Only produces ~0.3% performance bump on parser benchmarks. But, why
not?...
As discussed on #2046, it wasn't ideal to have `unsafe {
lexer.consume_ascii_char() }` in every byte handler. It also wasn't
great to have a safe function `consume_ascii_char()` which could cause
UB if called incorrectly (so wasn't really safe at all).
This PR achieves the same objective of #2046, but using a macro to
define byte handlers for ASCII chars, which builds in the assertion that
next char is guaranteed to be ASCII.
Before #2046:
```rs
const SPS: ByteHandler = |lexer| {
lexer.consume_char();
Kind::WhiteSpace
};
```
After this PR:
```rs
ascii_byte_handler!(SPS(lexer) {
lexer.consume_char();
Kind::WhiteSpace
});
```
i.e. The body of the handlers are unchanged from how they were before
https://github.com/oxc-project/oxc/pull/2046.
This expands to:
```rs
const SPS: ByteHandler = |lexer| {
unsafe {
let s = lexer.current.chars.as_str();
assert_unchecked!(!s.is_empty());
assert_unchecked!(s.as_bytes()[0] < 128);
}
lexer.consume_char();
Kind::WhiteSpace
};
```
But due to the assertions the macro inserts, `consume_char()` is now
optimized for ASCII characters, and reduces to a single instruction. So
the `consume_ascii_char()` function introduced by #2046 is unnecessary,
and can be removed again.
The "boundary of unsafe" is moved to a new function `handle_byte()`
which `read_next_token()` calls. `read_next_token()` is responsible for
upholding the safety invariants, which include ensuring that
`ascii_byte_handler!()` macro is not being misused (that last part is
strictly speaking a bit of a cheat, but...).
I am not a fan of macros, as they're not great for readability. But in
this case I don't think it's *too* bad, because:
1. The macro is well-documented.
2. It's not too clever (only one syntax is accepted).
3. It's used repetitively in a clear pattern, and once you've understood
one, you understand them all.
What do you think? Does this strike a reasonable balance between
readability and safety?
In the lexer, most `BYTE_HANDLER`s immediately consume the current char
with `lexer.consume_char()`.
Byte handlers are only called if there's a certain value (or range of
values) for the next char. This is their entire purpose. So in all cases
we know for sure that we're not at EOF, and that the next char is a
single-byte ASCII character.
The compiler, however, doesn't seem to be able to "see through" the
`BYTE_HANDLERS[byte](self)` call and understand these invariants. So it
produces very verbose ASM for `lexer.consume_char()`.
This PR replaces `lexer.consume_char()` in the byte handlers with an
unsafe `lexer.consume_ascii_char()` which skips on to next char with a
single `inc` instruction.
The difference in codegen can be seen here:
https://godbolt.org/z/1ha3cr9W5 (compare the 2 x
`core::ops::function::FnOnce::call_once` handlers).
Downside is that this does introduce a lot of unsafe blocks, but in my
opinion they're all pretty trivial to validate.
---------
Co-authored-by: Boshen <boshenc@gmail.com>
2 related changes to lexer's `read_next_token()`:
1. Hint to branch predictor that unicode identifiers and non-standard
whitespace are rare by marking that branch `#[cold]`.
2. The branch is on whether next character is ASCII or not. This check
only requires reading 1 byte, as ASCII characters are always single byte
in UTF8. So only do the work of getting a `char` in the cold path, once
it's established that character is not ASCII and this work is required.
This PR removes some code in parsing regexp flags which is extraneous:
```rs
if !ch.is_ascii_lowercase() {
self.error(diagnostics::RegExpFlag(ch, self.current_offset()));
continue;
}
```
Which is followed by:
```rs
let flag = if let Ok(flag) = RegExpFlags::try_from(ch) {
flag
} else {
self.error(diagnostics::RegExpFlag(ch, self.current_offset()));
continue;
};
```
`!ch.is_ascii_lowercase()` is equivalent to `ch < 'a' || ch > 'z'`. The
compiler implements `RegExpFlags::try_from(ch)` as `ch < 'd' || ch >
'y'` and then a jump table. So `ch.is_ascii_lowercase()` does nothing
that `RegExpFlags::try_from(ch)` doesn't do already.
https://godbolt.org/z/51GPPY9nx
(this PR built on top of #2007 for ease)
Part of #1880
`Token` size is reduced from 32 to 16 bytes by changing the previous
token value `Option<&'a str>` to a u32 index handle.
It would be nice if this handle is eliminated entirely because
the normal case for a string is always
`&source_text[token.span.start.token.span.end]`
Unfortunately, JavaScript allows escaped characters to appear in
identifiers, strings and templates. These strings need to be unescaped
for equality checks, i.e. `"\a" === "a"`.
This leads us to adding a `escaped_strings[]` vec for storing these
unescaped and allocated
strings.
Performance regression for adding this vec should be minimal because
escaped strings are rare.
Background Reading:
* https://floooh.github.io/2018/06/17/handles-vs-pointers.html
This PR is part of #1880.
`Token` size is reduced from 48 to 40 bytes.
To reconstruct the regex pattern and flags within the parser , the regex
string is
re-parsed from the end by reading all valid flags.
In order to make things work nicely, the lexer will no longer recover
from a invalid regex.
This PR partially fixes#1803 and is part of #1880.
BigInt is removed from the `Token` value, so that the token size can be
reduced once we removed all the variants.
`Token` is now also `Copy`, which removes all the `clone` and `drop`
calls.
This yields 5% performance improvement for the parser.
Parser incorrectly identifies string literals as directives if they
follow after `import`s, `export`s, or decorators.
In all of these cases, `'use strict'` produces a directive in the AST,
where it should be parsed as an `ExpressionStatement` containing a
`StringLiteral`:
```js
import x from 'foo';
'use strict';
```
```js
export {x};
'use strict';
```
```js
@foo
'use strict';
```
[Playground](https://oxc-project.github.io/oxc/playground/?code=3YCAAIC0gICAgICAgIC0G8rnONK89ITJ3zrK%2FUP7OmSZPgHQzStr3yMtwFTU%2BD1WPt09JgqZJLoYooydbGsM5vGcf34BnIA%3D)
This PR should fix that.
I'm not sure about the decorator case, though. I assume it's not a
directive. But is prefixing a string literal with a decorator even legal
syntax anyway?
And a side nit: If I'm reading it right, I don't think the `continue`
statement in the decorator arm of the match does anything. Do I have
that right?
Last question: Where does one go about putting a test? I guess these
silly cases aren't covered by Babel etc's tests.
---------
Co-authored-by: Boshen <boshenc@gmail.com>
`Token` and `Span` both represent `start` and `end` as `u32`.
This limits size of source which can be parsed to `u32::MAX`.
19577709db/crates/oxc_span/src/span.rs (L14-L20)
However, this constraint is currently not enforced.
In a release build, code will not panic on arithmetic overflow, so
`start`/`end` could wrap around back to zero if source is 4 GiB or more.
That'd produce nonsense spans. But worse, the lexer relies in some
places on `self.current.token.start` being correct, so if the value
wrapped around, possibly it'd keep rewinding to the start of the source
and lexing it again, causing an infinite loop.
In worst case, if for some reason an application's public API used OXC's
parser with user-supplied source code (parser-as-a-service!), this could
be exploited for denial of service.
This PR adds an assertion to catch this at the start of parsing instead.
This does add an extra instruction, but I imagine the effect will be
negligible compared to the work required to parse the code.
Parser, trivias and trivias_builder were edited to get all whitespaces.
Now Trivias struct store comments and whitespaces Vec. After that, i
will implement the no-irregular-whitespace rule.
P.S.: There isn't a way to implement this feature without lose a little
bit of performance, comparing with my last PR #1819 to minimax this
trouble instead of store the irregular whitespace as Span it was stored
as u32, i removed a map iterator and removed too a unused function. If
you have a suggestion about it pls give me a feedback.
Just removes a couple of lines of redundant code from the lexer.
A note on the 2nd one:
```rs
let mut builder = AutoCow::new(lexer);
let c = lexer.consume_char();
builder.push_matching(c);
```
`push_matching()` is a no-op unless
`force_allocation_without_current_ascii_char()` has already been called.
Here the `AutoCow` has just been freshly created, so we know it hasn't.
Most TypeScript types can be eliminated during the code generation phase
by not printing the corresponding AST nodes.
The changes in this PR enable applying a similar technique to the `this`
parameter.
The ECMA specification seems to added the "Tokens" section to the
specification as 12.6. This pushed all the other sections down,
resulting in e.g. former 12.6 now being 12.7. Comments in the parser
mention this part of the specification. All the mentions of section
12.6+ therefor are outdated now. This pull request tries to fix that by
updating all the comments.
