-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Span is currently defined as:
#[repr(C)]
pub struct Span {
pub start: u32,
pub end: u32,
_align: PointerAlign,
}
#[repr(transparent)]
struct PointerAlign([usize; 0]);PointerAlign ensures that Span is aligned on 8 on 64-bit systems. Some methods make use of this property to perform faster operations on Spans e.g. PartialEq compares 2 Spans by converting them to u64s first, which is faster.
Background
I had been scratching my head about why oxc-project/oxc#10933, which converted lexer's Token to a single u128 had such a large perf benefit. We'd also tried laying out the struct's fields so it was identical in memory to what that PR did, but that did not provide the same perf boost.
Now, after conversation on oxc-project/oxc#13042 (review), I think I have a guess as to why. That PR converted Token to:
#[derive(Clone, Copy)]
#[repr(transparent)]
pub struct Token(u128);The key to the perf boost might have been #[repr(transparent)]. Maybe that made Token able to be passed to functions in a register, whereas the multi-field struct couldn't.
How this applies to Span
I thought that aligning Span on 8 would make it able to passed to functions in a single register, rather than 2 registers for each field. But now I'm not so sure.
Maybe we could do the same trick as on Token, and convert Span to:
#[derive(Clone, Copy)]
#[repr(transparent)]
pub struct Span(u64);
impl Span {
pub fn start(self) -> u32 {
self.0 as u32
}
pub fn end(self) -> u32 {
self.0 >> 32 as u32
}
}As Spans are commonly copied around, this might give a measurable benefit.
Battle plan
First of all, could test the theory that Token's good perf is due to #[repr(transparent)] by removing that attribute (or replacing it with #[repr(C)]) and seeing what effect is has on benchmarks.
If that does confirm the theory, then alter Span accordingly.
32-bit platforms
On 32-bit platforms e.g. WASM, it may be beneficial for Span to remain 2 x u32s. On the other hand, even though WASM is 32 bit, the underlying system is likely to be 64 bit, so representing Span as a u64 may still be beneficial in WASM.
Other 32-bit systems exist, and Oxc nominally supports them. But in practice I doubt we have any users running Oxc on old 32-bit computers, so WASM is the only real 32-bit target.
Metadata
Metadata
Assignees
Labels
Type
Fields
Give feedbackPriority