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.

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

Decide on i128/u128 in Value Type #143

Closed
mitsuhiko opened this issue Nov 10, 2022 · 1 comment · Fixed by #144
Closed

Decide on i128/u128 in Value Type #143

mitsuhiko opened this issue Nov 10, 2022 · 1 comment · Fixed by #144

Comments

@mitsuhiko
Copy link
Owner

mitsuhiko commented Nov 10, 2022

With #142 the size of the Value type increased as i128/u128 are now inlined rather than in an Arc. This also increases the size of an Instruction from 32 bytes to 48 bytes on mac. For some reason the sizes appear to still be as expected on Linux (according to GHA at least).

The changes from #142 entirely results in noticeable performance differences for parse and compile (regression) but and render (improvements):

cmp_compile/minijinja   time:   [6.6548 µs 6.6674 µs 6.6817 µs]
                        change: [+1.2169% +1.5215% +1.8229%] (p = 0.00 < 0.05)
                        Performance has regressed.

cmp_render/minijinja    time:   [6.0559 µs 6.0681 µs 6.0793 µs]
                        change: [-3.8201% -3.5456% -3.2949%] (p = 0.00 < 0.05)
                        Performance has improved.

parse                   time:   [8.1028 µs 8.1185 µs 8.1328 µs]
                        change: [+1.1411% +1.3896% +1.6345%] (p = 0.00 < 0.05)
                        Performance has regressed.

compile                 time:   [12.986 µs 13.019 µs 13.054 µs]
                        change: [+1.3967% +1.6597% +1.9237%] (p = 0.00 < 0.05)
                        Performance has regressed.
@mitsuhiko
Copy link
Owner Author

mitsuhiko commented Nov 10, 2022

So looks like on x86_64 the thing still fits into the original sizes, on aarch64 it doesn't. That seems to be because the i128/u128 alignment is 16 on aarch64 vs 8 on x86_64.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant