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

[Merged by Bors] - Removed some unsafe_empty_trace!() calls to improve performance #2233

Closed
wants to merge 3 commits into from

Conversation

Razican
Copy link
Member

@Razican Razican commented Aug 13, 2022

This Pull Request fixes #1615.

It changes the following:

  • Removes the Trace implementation from types that don't need it (except for JsSymbol and JsString, which are needed elsewere).
  • Uses #[unsafe_ignore_trace] in places where we need to implement Trace for part of a structure.
  • Implements a custom Trace in enums where deriving it is not possible, since #[unsafe_ignore_trace] doesn't work for enums.

@Razican Razican added performance Performance related changes and issues Internal Category for changelog labels Aug 13, 2022
@Razican Razican added this to the v0.16.0 milestone Aug 13, 2022
@github-actions
Copy link

github-actions bot commented Aug 13, 2022

Test262 conformance changes

VM implementation

Test result main count PR count difference
Total 91,733 91,733 0
Passed 64,830 64,830 0
Ignored 14,750 14,750 0
Failed 12,153 12,153 0
Panics 0 0 0
Conformance 70.67% 70.67% 0.00%

@codecov
Copy link

codecov bot commented Aug 13, 2022

Codecov Report

Merging #2233 (8df810e) into main (d5312f5) will decrease coverage by 0.03%.
The diff coverage is 77.77%.

@@            Coverage Diff             @@
##             main    #2233      +/-   ##
==========================================
- Coverage   41.35%   41.32%   -0.04%     
==========================================
  Files         234      234              
  Lines       22009    22023      +14     
==========================================
- Hits         9101     9100       -1     
- Misses      12908    12923      +15     
Impacted Files Coverage Δ
boa_engine/src/bigint.rs 38.34% <ø> (ø)
boa_engine/src/builtins/array/array_iterator.rs 86.66% <ø> (ø)
boa_engine/src/builtins/date/mod.rs 81.15% <ø> (-0.25%) ⬇️
boa_engine/src/builtins/function/mod.rs 23.71% <ø> (-0.40%) ⬇️
boa_engine/src/builtins/map/map_iterator.rs 94.11% <ø> (ø)
boa_engine/src/builtins/regexp/mod.rs 67.01% <ø> (-1.23%) ⬇️
boa_engine/src/builtins/set/set_iterator.rs 80.00% <ø> (ø)
...src/builtins/typed_array/integer_indexed_object.rs 0.00% <ø> (ø)
boa_engine/src/builtins/typed_array/mod.rs 3.76% <ø> (ø)
boa_engine/src/property/attribute/mod.rs 0.00% <ø> (ø)
... and 43 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@raskad raskad left a comment

Choose a reason for hiding this comment

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

Nice work, great to have this cleaned up.

boa_engine/src/builtins/array/array_iterator.rs Outdated Show resolved Hide resolved
@Razican Razican added the run-benchmark Label used to run banchmarks on PRs label Aug 13, 2022
@github-actions
Copy link

Benchmark for 3a3a607

Click to view benchmark
Test Base PR %
Arithmetic operations (Compiler) 512.8±0.83ns 523.4±0.92ns +2.07%
Arithmetic operations (Execution) 660.3±0.65ns 710.7±14.89ns +7.63%
Arithmetic operations (Parser) 6.5±0.01µs 6.4±0.01µs -1.54%
Array access (Compiler) 1384.2±6.52ns 1382.7±6.56ns -0.11%
Array access (Execution) 7.9±0.09µs 7.8±0.02µs -1.27%
Array access (Parser) 13.7±0.02µs 13.5±0.02µs -1.46%
Array creation (Compiler) 2.1±0.01µs 2.1±0.00µs 0.00%
Array creation (Execution) 1082.0±5.01µs 1084.0±6.32µs +0.18%
Array creation (Parser) 15.8±0.02µs 15.6±0.03µs -1.27%
Array pop (Compiler) 4.2±0.02µs 4.0±0.01µs -4.76%
Array pop (Execution) 569.9±3.09µs 572.8±1.70µs +0.51%
Array pop (Parser) 149.2±0.18µs 148.8±0.23µs -0.27%
Boolean Object Access (Compiler) 1099.3±2.72ns 1112.0±1.80ns +1.16%
Boolean Object Access (Execution) 4.1±0.01µs 4.2±0.01µs +2.44%
Boolean Object Access (Parser) 15.7±0.03µs 15.8±0.03µs +0.64%
Clean js (Compiler) 4.5±0.02µs 4.5±0.02µs 0.00%
Clean js (Execution) 610.9±3.75µs 608.1±2.30µs -0.46%
Clean js (Parser) 32.7±0.06µs 32.5±0.03µs -0.61%
Create Realm 229.5±3.04ns 228.3±0.20ns -0.52%
Dynamic Object Property Access (Compiler) 1632.9±7.25ns 1649.5±7.44ns +1.02%
Dynamic Object Property Access (Execution) 5.2±0.02µs 5.2±0.01µs 0.00%
Dynamic Object Property Access (Parser) 12.4±0.02µs 12.2±0.06µs -1.61%
Fibonacci (Compiler) 2.6±0.01µs 2.6±0.01µs 0.00%
Fibonacci (Execution) 1290.9±3.95µs 1279.0±3.20µs -0.92%
Fibonacci (Parser) 18.6±0.04µs 18.3±0.02µs -1.61%
For loop (Compiler) 2.4±0.01µs 2.4±0.01µs 0.00%
For loop (Execution) 16.6±0.04µs 16.7±0.05µs +0.60%
For loop (Parser) 16.2±0.02µs 16.2±0.02µs 0.00%
Mini js (Compiler) 4.0±0.01µs 4.0±0.01µs 0.00%
Mini js (Execution) 556.4±5.10µs 548.9±3.08µs -1.35%
Mini js (Parser) 28.5±0.05µs 28.4±0.04µs -0.35%
Number Object Access (Compiler) 1039.7±1.49ns 1047.7±2.30ns +0.77%
Number Object Access (Execution) 3.2±0.00µs 3.2±0.01µs 0.00%
Number Object Access (Parser) 12.2±0.01µs 12.2±0.02µs 0.00%
Object Creation (Compiler) 1461.1±2.44ns 1437.4±16.38ns -1.62%
Object Creation (Execution) 4.9±0.02µs 4.9±0.02µs 0.00%
Object Creation (Parser) 10.8±0.02µs 10.6±0.01µs -1.85%
RegExp (Compiler) 1635.3±7.49ns 1622.6±7.33ns -0.78%
RegExp (Execution) 12.2±0.04µs 11.7±0.03µs -4.10%
RegExp (Parser) 11.8±0.02µs 11.7±0.03µs -0.85%
RegExp Creation (Compiler) 1469.0±2.40ns 1466.3±7.66ns -0.18%
RegExp Creation (Execution) 9.1±0.04µs 8.7±0.05µs -4.40%
RegExp Creation (Parser) 9.9±0.02µs 9.8±0.03µs -1.01%
RegExp Literal (Compiler) 1615.7±5.50ns 1613.7±9.73ns -0.12%
RegExp Literal (Execution) 12.1±0.06µs 11.6±0.04µs -4.13%
RegExp Literal (Parser) 9.6±0.10µs 9.5±0.02µs -1.04%
RegExp Literal Creation (Compiler) 1463.7±1.83ns 1468.8±2.86ns +0.35%
RegExp Literal Creation (Execution) 9.2±0.14µs 8.7±0.04µs -5.43%
RegExp Literal Creation (Parser) 7.5±0.02µs 7.4±0.02µs -1.33%
Static Object Property Access (Compiler) 1454.4±3.16ns 1463.7±1.96ns +0.64%
Static Object Property Access (Execution) 5.1±0.02µs 5.0±0.02µs -1.96%
Static Object Property Access (Parser) 11.5±0.02µs 11.3±0.02µs -1.74%
String Object Access (Compiler) 1368.4±6.88ns 1400.0±4.38ns +2.31%
String Object Access (Execution) 6.0±0.01µs 6.0±0.02µs 0.00%
String Object Access (Parser) 15.2±0.01µs 15.2±0.08µs 0.00%
String comparison (Compiler) 2.1±0.01µs 2.2±0.01µs +4.76%
String comparison (Execution) 4.5±0.01µs 4.4±0.03µs -2.22%
String comparison (Parser) 12.4±0.01µs 12.5±0.02µs +0.81%
String concatenation (Compiler) 1635.4±7.77ns 1644.7±7.76ns +0.57%
String concatenation (Execution) 4.3±0.04µs 4.2±0.02µs -2.33%
String concatenation (Parser) 8.7±0.01µs 8.8±0.02µs +1.15%
String copy (Compiler) 1319.7±4.03ns 1332.4±3.57ns +0.96%
String copy (Execution) 4.0±0.02µs 4.0±0.02µs 0.00%
String copy (Parser) 6.6±0.02µs 6.5±0.05µs -1.52%
Symbols (Compiler) 991.4±2.00ns 1018.3±3.78ns +2.71%
Symbols (Execution) 4.1±0.02µs 4.0±0.01µs -2.44%
Symbols (Parser) 5.1±0.05µs 5.1±0.01µs 0.00%

@HalidOdat
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Aug 14, 2022
<!---
Thank you for contributing to Boa! Please fill out the template below, and remove or add any
information as you feel neccesary.
--->

This Pull Request fixes #1615.

It changes the following:

- Removes the `Trace` implementation from types that don't need it (except for `JsSymbol` and `JsString`, which are needed elsewere).
- Uses `#[unsafe_ignore_trace]` in places where we need to implement `Trace` for part of a structure.
- Implements a custom `Trace` in enums where deriving it is not possible, since `#[unsafe_ignore_trace]` doesn't work for enums.
@bors
Copy link

bors bot commented Aug 14, 2022

Build failed:

@Razican
Copy link
Member Author

Razican commented Aug 15, 2022

Bors retry

bors bot pushed a commit that referenced this pull request Aug 15, 2022
<!---
Thank you for contributing to Boa! Please fill out the template below, and remove or add any
information as you feel neccesary.
--->

This Pull Request fixes #1615.

It changes the following:

- Removes the `Trace` implementation from types that don't need it (except for `JsSymbol` and `JsString`, which are needed elsewere).
- Uses `#[unsafe_ignore_trace]` in places where we need to implement `Trace` for part of a structure.
- Implements a custom `Trace` in enums where deriving it is not possible, since `#[unsafe_ignore_trace]` doesn't work for enums.
@bors
Copy link

bors bot commented Aug 15, 2022

Build failed:

@github-actions
Copy link

Benchmark for 9e4a166

Click to view benchmark
Test Base PR %
Arithmetic operations (Compiler) 670.1±26.93ns 658.3±28.24ns -1.76%
Arithmetic operations (Execution) 754.6±27.97ns 752.2±32.24ns -0.32%
Arithmetic operations (Parser) 7.4±0.28µs 7.2±0.42µs -2.70%
Array access (Compiler) 1769.6±142.20ns 1772.3±103.88ns +0.15%
Array access (Execution) 9.2±0.31µs 9.4±0.74µs +2.17%
Array access (Parser) 15.3±0.76µs 14.2±0.64µs -7.19%
Array creation (Compiler) 2.7±0.11µs 2.6±0.12µs -3.70%
Array creation (Execution) 1203.9±41.31µs 1221.8±44.48µs +1.49%
Array creation (Parser) 18.3±0.84µs 16.9±0.57µs -7.65%
Array pop (Compiler) 4.7±0.19µs 4.6±0.26µs -2.13%
Array pop (Execution) 640.4±34.79µs 638.4±23.98µs -0.31%
Array pop (Parser) 180.2±28.82µs 178.3±10.67µs -1.05%
Boolean Object Access (Compiler) 1303.8±56.59ns 1302.4±64.30ns -0.11%
Boolean Object Access (Execution) 5.3±0.19µs 5.2±0.26µs -1.89%
Boolean Object Access (Parser) 18.1±0.96µs 16.5±0.74µs -8.84%
Clean js (Compiler) 5.5±0.32µs 5.6±0.29µs +1.82%
Clean js (Execution) 685.7±33.43µs 692.1±23.13µs +0.93%
Clean js (Parser) 37.4±1.64µs 35.7±1.76µs -4.55%
Create Realm 280.9±13.10ns 283.0±16.17ns +0.75%
Dynamic Object Property Access (Compiler) 2.1±0.15µs 2.0±0.11µs -4.76%
Dynamic Object Property Access (Execution) 6.5±0.30µs 6.4±0.40µs -1.54%
Dynamic Object Property Access (Parser) 14.1±0.67µs 13.3±1.07µs -5.67%
Fibonacci (Compiler) 3.3±0.30µs 3.3±0.25µs 0.00%
Fibonacci (Execution) 1583.7±67.75µs 1563.8±50.00µs -1.26%
Fibonacci (Parser) 21.8±1.38µs 20.2±1.22µs -7.34%
For loop (Compiler) 3.0±0.16µs 3.1±0.14µs +3.33%
For loop (Execution) 20.2±0.77µs 20.3±2.03µs +0.50%
For loop (Parser) 19.6±1.29µs 18.1±1.78µs -7.65%
Mini js (Compiler) 4.9±0.24µs 4.9±0.55µs 0.00%
Mini js (Execution) 631.0±45.63µs 622.5±26.12µs -1.35%
Mini js (Parser) 32.9±1.36µs 30.9±1.12µs -6.08%
Number Object Access (Compiler) 1247.0±49.31ns 1227.8±64.14ns -1.54%
Number Object Access (Execution) 4.3±0.28µs 4.1±0.25µs -4.65%
Number Object Access (Parser) 13.9±0.86µs 13.7±0.88µs -1.44%
Object Creation (Compiler) 1884.6±92.01ns 1790.6±83.20ns -4.99%
Object Creation (Execution) 6.2±0.40µs 6.1±0.29µs -1.61%
Object Creation (Parser) 12.4±0.98µs 11.2±0.50µs -9.68%
RegExp (Compiler) 2.1±0.08µs 2.1±0.12µs 0.00%
RegExp (Execution) 14.2±0.68µs 14.3±1.37µs +0.70%
RegExp (Parser) 13.3±0.82µs 12.6±0.98µs -5.26%
RegExp Creation (Compiler) 1852.2±81.38ns 1875.6±104.37ns +1.26%
RegExp Creation (Execution) 10.7±0.80µs 10.4±0.42µs -2.80%
RegExp Creation (Parser) 11.0±0.39µs 10.2±0.40µs -7.27%
RegExp Literal (Compiler) 2.1±0.09µs 2.1±0.13µs 0.00%
RegExp Literal (Execution) 14.3±0.69µs 13.8±0.48µs -3.50%
RegExp Literal (Parser) 10.8±0.57µs 10.1±0.54µs -6.48%
RegExp Literal Creation (Compiler) 1849.3±78.68ns 1843.9±87.75ns -0.29%
RegExp Literal Creation (Execution) 10.6±0.86µs 10.3±0.39µs -2.83%
RegExp Literal Creation (Parser) 8.3±0.37µs 8.0±0.50µs -3.61%
Static Object Property Access (Compiler) 1898.2±105.69ns 1833.9±61.29ns -3.39%
Static Object Property Access (Execution) 6.4±0.30µs 6.3±0.29µs -1.56%
Static Object Property Access (Parser) 13.0±0.85µs 12.0±0.90µs -7.69%
String Object Access (Compiler) 1652.9±89.24ns 1633.1±67.37ns -1.20%
String Object Access (Execution) 7.3±0.30µs 7.7±0.81µs +5.48%
String Object Access (Parser) 17.1±0.66µs 16.3±0.99µs -4.68%
String comparison (Compiler) 2.7±0.17µs 2.7±0.18µs 0.00%
String comparison (Execution) 5.5±0.30µs 5.5±0.28µs 0.00%
String comparison (Parser) 14.0±0.61µs 13.6±0.81µs -2.86%
String concatenation (Compiler) 2.1±0.21µs 2.0±0.07µs -4.76%
String concatenation (Execution) 5.4±0.29µs 5.1±0.29µs -5.56%
String concatenation (Parser) 9.8±0.45µs 9.3±0.43µs -5.10%
String copy (Compiler) 1698.7±84.10ns 1735.2±78.21ns +2.15%
String copy (Execution) 5.0±0.29µs 5.1±0.28µs +2.00%
String copy (Parser) 7.3±0.31µs 7.5±0.81µs +2.74%
Symbols (Compiler) 1366.8±94.80ns 1347.0±65.19ns -1.45%
Symbols (Execution) 5.3±0.31µs 4.8±0.16µs -9.43%
Symbols (Parser) 5.9±0.45µs 5.5±0.25µs -6.78%

@Razican
Copy link
Member Author

Razican commented Aug 17, 2022

bors r+

bors bot pushed a commit that referenced this pull request Aug 17, 2022
<!---
Thank you for contributing to Boa! Please fill out the template below, and remove or add any
information as you feel neccesary.
--->

This Pull Request fixes #1615.

It changes the following:

- Removes the `Trace` implementation from types that don't need it (except for `JsSymbol` and `JsString`, which are needed elsewere).
- Uses `#[unsafe_ignore_trace]` in places where we need to implement `Trace` for part of a structure.
- Implements a custom `Trace` in enums where deriving it is not possible, since `#[unsafe_ignore_trace]` doesn't work for enums.

Co-authored-by: raskad <32105367+raskad@users.noreply.github.com>
@bors
Copy link

bors bot commented Aug 17, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Removed some unsafe_empty_trace!() calls to improve performance [Merged by Bors] - Removed some unsafe_empty_trace!() calls to improve performance Aug 17, 2022
@bors bors bot closed this Aug 17, 2022
@bors bors bot deleted the remove_trace branch August 17, 2022 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internal Category for changelog performance Performance related changes and issues run-benchmark Label used to run banchmarks on PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cleanup invalid derive(Trace)
3 participants