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] - Unwrap removal #1842

Closed
wants to merge 3 commits into from
Closed

[Merged by Bors] - Unwrap removal #1842

wants to merge 3 commits into from

Conversation

Razican
Copy link
Member

@Razican Razican commented Feb 16, 2022

This removes all the calls to unwrap() in the codebase, which made me found a couple of places where it wasn't needed, and could be improved. I also noticed we don't have dependabot updates for the test262 submodule and the interner dependencies, so I added those.

I added lints so that no new unwraps are added.

@Razican Razican added enhancement New feature or request dependencies Pull requests that update a dependency file Internal Category for changelog labels Feb 16, 2022
@Razican Razican added this to the v0.14.0 milestone Feb 16, 2022
@github-actions
Copy link

Test262 conformance changes

VM implementation

Test result main count PR count difference
Total 87,912 87,912 0
Passed 40,956 40,956 0
Ignored 21,153 21,153 0
Failed 25,803 25,803 0
Panics 0 0 0
Conformance 46.59% 46.59% 0.00%

@codecov
Copy link

codecov bot commented Feb 16, 2022

Codecov Report

Merging #1842 (34b446f) into main (51537ba) will decrease coverage by 0.06%.
The diff coverage is 60.60%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1842      +/-   ##
==========================================
- Coverage   55.67%   55.60%   -0.07%     
==========================================
  Files         201      201              
  Lines       17368    17397      +29     
==========================================
+ Hits         9670     9674       +4     
- Misses       7698     7723      +25     
Impacted Files Coverage Δ
boa/src/builtins/iterable/mod.rs 65.38% <ø> (ø)
boa/src/builtins/object/mod.rs 65.87% <0.00%> (ø)
boa/src/lib.rs 87.09% <ø> (ø)
boa/src/object/jsobject.rs 64.70% <0.00%> (-1.17%) ⬇️
boa/src/syntax/parser/expression/mod.rs 77.77% <ø> (+0.38%) ⬆️
boa_cli/src/helper.rs 0.00% <ø> (ø)
boa_cli/src/main.rs 5.26% <0.00%> (-0.99%) ⬇️
boa_interner/src/lib.rs 50.00% <ø> (ø)
boa_tester/src/main.rs 0.00% <ø> (ø)
boa_unicode/src/lib.rs 69.69% <ø> (ø)
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51537ba...34b446f. Read the comment docs.

@github-actions
Copy link

Benchmark for 6b67b13

Click to view benchmark
Test Base PR %
Arithmetic operations (Compiler) 402.6±2.37ns 405.9±4.28ns +0.82%
Arithmetic operations (Execution) 2.0±0.00µs 2.0±0.02µs 0.00%
Arithmetic operations (Parser) 5.5±0.02µs 5.5±0.00µs 0.00%
Array access (Compiler) 815.7±1.73ns 819.1±1.59ns +0.42%
Array access (Execution) 10.3±0.04µs 10.3±0.07µs 0.00%
Array access (Parser) 11.9±0.03µs 11.7±0.02µs -1.68%
Array creation (Compiler) 1167.7±2.83ns 1180.9±2.78ns +1.13%
Array creation (Execution) 3.3±0.02ms 3.2±0.01ms -3.03%
Array creation (Parser) 13.2±0.03µs 13.2±0.08µs 0.00%
Array pop (Compiler) 2.7±0.01µs 2.9±0.01µs +7.41%
Array pop (Execution) 1387.6±5.43µs 1363.3±3.93µs -1.75%
Array pop (Parser) 137.8±0.11µs 135.6±0.12µs -1.60%
Boolean Object Access (Compiler) 691.1±0.96ns 696.9±1.33ns +0.84%
Boolean Object Access (Execution) 7.1±0.02µs 6.9±0.02µs -2.82%
Boolean Object Access (Parser) 14.0±0.01µs 14.0±0.03µs 0.00%
Clean js (Compiler) 2.2±0.00µs 2.2±0.00µs 0.00%
Clean js (Execution) 1497.0±7.98µs 1473.1±25.37µs -1.60%
Clean js (Parser) 28.5±0.05µs 28.3±0.10µs -0.70%
Create Realm 335.7±3.33ns 341.4±0.13ns +1.70%
Dynamic Object Property Access (Compiler) 1166.0±3.48ns 1183.3±4.17ns +1.48%
Dynamic Object Property Access (Execution) 6.7±0.03µs 6.9±0.03µs +2.99%
Dynamic Object Property Access (Parser) 10.7±0.02µs 10.6±0.02µs -0.93%
Fibonacci (Compiler) 1467.1±3.07ns 1471.2±3.38ns +0.28%
Fibonacci (Execution) 2.7±0.00ms 2.8±0.00ms +3.70%
Fibonacci (Parser) 16.0±0.02µs 15.8±0.02µs -1.25%
For loop (Compiler) 1282.0±2.79ns 1293.1±2.47ns +0.87%
For loop (Execution) 44.0±0.14µs 43.6±0.19µs -0.91%
For loop (Parser) 13.7±0.05µs 13.7±0.12µs 0.00%
Mini js (Compiler) 2.1±0.00µs 2.1±0.00µs 0.00%
Mini js (Execution) 1383.9±7.34µs 1360.0±8.53µs -1.73%
Mini js (Parser) 25.2±0.03µs 24.7±0.05µs -1.98%
Number Object Access (Compiler) 654.5±1.10ns 649.2±1.32ns -0.81%
Number Object Access (Execution) 5.7±0.01µs 5.4±0.01µs -5.26%
Number Object Access (Parser) 11.0±0.01µs 11.0±0.03µs 0.00%
Object Creation (Compiler) 959.3±2.16ns 971.2±3.16ns +1.24%
Object Creation (Execution) 6.1±0.02µs 6.2±0.02µs +1.64%
Object Creation (Parser) 9.3±0.04µs 9.1±0.02µs -2.15%
RegExp (Compiler) 1140.1±1.65ns 1164.6±3.27ns +2.15%
RegExp (Execution) 12.6±0.03µs 12.9±0.03µs +2.38%
RegExp (Parser) 10.0±0.02µs 9.9±0.02µs -1.00%
RegExp Creation (Compiler) 1041.0±3.49ns 1029.4±3.13ns -1.11%
RegExp Creation (Execution) 9.5±0.05µs 9.9±0.05µs +4.21%
RegExp Creation (Parser) 8.3±0.03µs 8.3±0.07µs 0.00%
RegExp Literal (Compiler) 1165.9±3.79ns 1150.9±2.56ns -1.29%
RegExp Literal (Execution) 12.6±0.03µs 13.0±0.03µs +3.17%
RegExp Literal (Parser) 8.1±0.02µs 8.0±0.02µs -1.23%
RegExp Literal Creation (Compiler) 1039.8±3.51ns 1045.1±3.21ns +0.51%
RegExp Literal Creation (Execution) 9.5±0.05µs 10.0±0.06µs +5.26%
RegExp Literal Creation (Parser) 6.4±0.01µs 6.3±0.01µs -1.56%
Static Object Property Access (Compiler) 997.5±4.40ns 994.3±4.20ns -0.32%
Static Object Property Access (Execution) 6.3±0.02µs 6.4±0.03µs +1.59%
Static Object Property Access (Parser) 9.9±0.02µs 9.8±0.03µs -1.01%
String Object Access (Compiler) 1041.8±6.43ns 1045.1±6.41ns +0.32%
String Object Access (Execution) 9.0±0.02µs 8.6±0.02µs -4.44%
String Object Access (Parser) 13.8±0.03µs 13.8±0.02µs 0.00%
String comparison (Compiler) 1295.7±12.39ns 1302.0±4.14ns +0.49%
String comparison (Execution) 7.1±0.03µs 7.2±0.03µs +1.41%
String comparison (Parser) 10.8±0.06µs 10.7±0.01µs -0.93%
String concatenation (Compiler) 1058.3±3.00ns 1078.1±3.82ns +1.87%
String concatenation (Execution) 6.1±0.01µs 6.1±0.02µs 0.00%
String concatenation (Parser) 7.5±0.04µs 7.4±0.03µs -1.33%
String copy (Compiler) 873.2±2.91ns 904.1±3.97ns +3.54%
String copy (Execution) 5.2±0.02µs 5.3±0.02µs +1.92%
String copy (Parser) 5.6±0.01µs 5.5±0.01µs -1.79%
Symbols (Compiler) 653.6±1.51ns 654.0±1.61ns +0.06%
Symbols (Execution) 4.9±0.02µs 4.9±0.02µs 0.00%
Symbols (Parser) 4.3±0.01µs 4.2±0.01µs -2.33%

Copy link
Member

@HalidOdat HalidOdat left a comment

Choose a reason for hiding this comment

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

Looks good to me! :)

@RageKnify
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Feb 17, 2022
This removes all the calls to  `unwrap()` in the codebase, which made me found a couple of places where it wasn't needed, and could be improved. I also noticed we don't have dependabot updates for the test262 submodule and the interner dependencies, so I added those.

I added lints so that no new unwraps are added.
@bors
Copy link

bors bot commented Feb 17, 2022

Build failed:

@Razican
Copy link
Member Author

Razican commented Feb 17, 2022

bors r+

@bors
Copy link

bors bot commented Feb 17, 2022

🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

bors bot pushed a commit that referenced this pull request Feb 17, 2022
This removes all the calls to  `unwrap()` in the codebase, which made me found a couple of places where it wasn't needed, and could be improved. I also noticed we don't have dependabot updates for the test262 submodule and the interner dependencies, so I added those.

I added lints so that no new unwraps are added.
@bors
Copy link

bors bot commented Feb 17, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Unwrap removal [Merged by Bors] - Unwrap removal Feb 17, 2022
@bors bors bot closed this Feb 17, 2022
@bors bors bot deleted the unwrap_removal branch February 17, 2022 17:55
@github-actions
Copy link

Benchmark for c5245c1

Click to view benchmark
Test Base PR %
Arithmetic operations (Compiler) 464.1±16.56ns 457.8±4.95ns -1.36%
Arithmetic operations (Execution) 2.3±0.07µs 2.2±0.09µs -4.35%
Arithmetic operations (Parser) 5.8±0.14µs 6.2±0.02µs +6.90%
Array access (Compiler) 1000.1±6.26ns 925.6±18.29ns -7.45%
Array access (Execution) 12.2±0.30µs 12.2±0.18µs 0.00%
Array access (Parser) 12.3±0.53µs 13.4±0.02µs +8.94%
Array creation (Compiler) 1381.2±18.59ns 1327.4±26.37ns -3.90%
Array creation (Execution) 3.7±0.08ms 3.8±0.03ms +2.70%
Array creation (Parser) 14.5±0.39µs 15.1±0.03µs +4.14%
Array pop (Compiler) 3.6±0.07µs 3.2±0.07µs -11.11%
Array pop (Execution) 1586.3±20.25µs 1641.2±15.64µs +3.46%
Array pop (Parser) 158.4±1.85µs 161.4±0.25µs +1.89%
Boolean Object Access (Compiler) 795.9±18.22ns 763.1±12.90ns -4.12%
Boolean Object Access (Execution) 7.9±0.26µs 8.1±0.18µs +2.53%
Boolean Object Access (Parser) 15.3±0.35µs 16.0±0.04µs +4.58%
Clean js (Compiler) 2.6±0.05µs 2.6±0.02µs 0.00%
Clean js (Execution) 1668.5±56.50µs 1656.1±69.72µs -0.74%
Clean js (Parser) 31.2±0.38µs 32.2±0.10µs +3.21%
Create Realm 365.9±20.36ns 391.1±1.66ns +6.89%
Dynamic Object Property Access (Compiler) 1368.6±20.22ns 1384.4±7.76ns +1.15%
Dynamic Object Property Access (Execution) 8.1±0.18µs 8.1±0.09µs 0.00%
Dynamic Object Property Access (Parser) 11.1±0.55µs 11.9±0.04µs +7.21%
Fibonacci (Compiler) 1743.6±33.35ns 1746.3±20.49ns +0.15%
Fibonacci (Execution) 3.2±0.10ms 3.2±0.05ms 0.00%
Fibonacci (Parser) 16.1±0.93µs 18.2±0.08µs +13.04%
For loop (Compiler) 1545.9±13.50ns 1512.6±9.32ns -2.15%
For loop (Execution) 50.1±1.40µs 51.9±0.93µs +3.59%
For loop (Parser) 13.5±0.76µs 15.9±0.08µs +17.78%
Mini js (Compiler) 2.6±0.03µs 2.5±0.05µs -3.85%
Mini js (Execution) 1584.9±42.67µs 1572.1±41.23µs -0.81%
Mini js (Parser) 27.3±0.30µs 28.1±0.14µs +2.93%
Number Object Access (Compiler) 763.3±9.40ns 721.9±16.73ns -5.42%
Number Object Access (Execution) 6.1±0.20µs 6.5±0.10µs +6.56%
Number Object Access (Parser) 11.8±0.25µs 12.4±0.04µs +5.08%
Object Creation (Compiler) 1136.3±22.61ns 1146.6±2.85ns +0.91%
Object Creation (Execution) 7.2±0.22µs 7.3±0.35µs +1.39%
Object Creation (Parser) 9.3±0.48µs 10.5±0.06µs +12.90%
RegExp (Compiler) 1365.3±8.43ns 1357.4±21.90ns -0.58%
RegExp (Execution) 14.6±0.46µs 15.3±0.22µs +4.79%
RegExp (Parser) 10.3±0.39µs 11.4±0.02µs +10.68%
RegExp Creation (Compiler) 1242.6±17.55ns 1220.9±9.28ns -1.75%
RegExp Creation (Execution) 10.8±0.33µs 11.6±0.17µs +7.41%
RegExp Creation (Parser) 9.0±0.16µs 9.5±0.02µs +5.56%
RegExp Literal (Compiler) 1352.5±16.86ns 1374.1±14.34ns +1.60%
RegExp Literal (Execution) 14.4±0.40µs 15.1±0.32µs +4.86%
RegExp Literal (Parser) 8.3±0.29µs 9.1±0.01µs +9.64%
RegExp Literal Creation (Compiler) 1217.8±16.71ns 1214.3±6.65ns -0.29%
RegExp Literal Creation (Execution) 11.0±0.36µs 11.6±0.19µs +5.45%
RegExp Literal Creation (Parser) 6.7±0.28µs 7.2±0.03µs +7.46%
Static Object Property Access (Compiler) 1159.0±15.76ns 1147.6±4.39ns -0.98%
Static Object Property Access (Execution) 7.7±0.13µs 7.4±0.17µs -3.90%
Static Object Property Access (Parser) 9.8±0.63µs 11.2±0.04µs +14.29%
String Object Access (Compiler) 1192.8±31.91ns 1211.0±13.31ns +1.53%
String Object Access (Execution) 10.1±0.21µs 10.2±0.22µs +0.99%
String Object Access (Parser) 15.1±0.32µs 15.7±0.04µs +3.97%
String comparison (Compiler) 1542.3±26.65ns 1495.8±25.34ns -3.01%
String comparison (Execution) 8.3±0.27µs 8.5±0.11µs +2.41%
String comparison (Parser) 11.5±0.36µs 12.5±0.05µs +8.70%
String concatenation (Compiler) 1253.8±31.78ns 1239.9±24.79ns -1.11%
String concatenation (Execution) 7.3±0.20µs 7.3±0.07µs 0.00%
String concatenation (Parser) 8.0±0.17µs 8.6±0.04µs +7.50%
String copy (Compiler) 1032.1±16.54ns 1019.4±18.38ns -1.23%
String copy (Execution) 6.2±0.14µs 6.3±0.06µs +1.61%
String copy (Parser) 6.0±0.09µs 6.4±0.02µs +6.67%
Symbols (Compiler) 762.9±10.26ns 768.3±4.15ns +0.71%
Symbols (Execution) 5.8±0.15µs 5.8±0.11µs 0.00%
Symbols (Parser) 4.3±0.20µs 4.8±0.02µs +11.63%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request Internal Category for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants