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

Value refactor #383

Merged
merged 2 commits into from
May 9, 2020
Merged

Value refactor #383

merged 2 commits into from
May 9, 2020

Conversation

HalidOdat
Copy link
Member

No description provided.

@github-actions
Copy link

github-actions bot commented May 8, 2020

Benchmark for e32802e

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 496.7±20.42µs 487.9±24.85µs 102%
Expression (Lexer) 1928.5±87.71ns 2.0±0.08µs 96%
Expression (Parser) 4.9±0.18µs 5.1±0.18µs 96%
Fibonacci (Execution) 3.4±0.12ms 3.4±0.10ms 101%
For loop (Execution) 500.8±20.55µs 522.0±35.92µs 96%
For loop (Lexer) 5.1±0.31µs 5.4±0.26µs 93%
For loop (Parser) 14.3±0.64µs 15.0±0.72µs 95%
Hello World (Lexer) 923.8±51.31ns 957.4±52.42ns 96%
Hello World (Parser) 2.3±0.15µs 2.3±0.12µs 103%
Symbols (Execution) 505.0±22.20µs 512.5±25.44µs 99%
undefined undefined 100%

Copy link
Member

@Razican Razican left a comment

Choose a reason for hiding this comment

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

Great job! this looks very good. I would divide the module in some submodules, since I think it's too big. The next big thing to do is to remove FromValue and ToValue traits and implement TryFrom and From traits.

boa/src/builtins/value/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/value/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/array/mod.rs Show resolved Hide resolved
@HalidOdat
Copy link
Member Author

The next big thing to do is to remove FromValue and ToValue traits and implement TryFrom and From traits.

Yeah! I already have some progress locally for From trait still needs some work though

@github-actions
Copy link

github-actions bot commented May 8, 2020

Benchmark for a47aeb3

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 448.9±20.22µs 462.7±13.38µs 97%
Expression (Lexer) 1825.2±44.68ns 1861.1±71.40ns 98%
Expression (Parser) 4.7±0.26µs 4.9±0.15µs 96%
Fibonacci (Execution) 3.0±0.10ms 3.1±0.08ms 97%
For loop (Execution) 443.6±18.28µs 477.3±10.65µs 92%
For loop (Lexer) 4.9±0.16µs 5.1±0.21µs 96%
For loop (Parser) 13.2±0.56µs 13.8±0.23µs 95%
Hello World (Lexer) 881.0±18.70ns 924.5±86.04ns 95%
Hello World (Parser) 2.2±0.06µs 2.2±0.07µs 96%
Symbols (Execution) 465.1±22.56µs 477.9±20.69µs 97%
undefined undefined 100%

@github-actions
Copy link

github-actions bot commented May 8, 2020

Benchmark for 3fe29bc

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 529.3±52.62µs 531.7±44.60µs 100%
Expression (Lexer) 2.0±0.14µs 2.2±0.20µs 90.99999999999999%
Expression (Parser) 5.3±0.36µs 5.4±0.26µs 99%
Fibonacci (Execution) 3.5±0.28ms 3.6±0.32ms 98%
For loop (Execution) 575.5±51.96µs 535.5±59.67µs 107%
For loop (Lexer) 5.6±0.37µs 5.7±0.51µs 98%
For loop (Parser) 14.6±1.13µs 15.7±1.44µs 92%
Hello World (Lexer) 1019.6±74.69ns 983.2±55.17ns 104%
Hello World (Parser) 2.4±0.13µs 2.4±0.15µs 97%
Symbols (Execution) 546.2±45.70µs 535.1±55.51µs 102%
undefined undefined 100%

@github-actions
Copy link

github-actions bot commented May 9, 2020

Benchmark for 5a25ce6

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 417.6±16.60µs 456.1±14.42µs 90.99999999999999%
Expression (Lexer) 1911.3±90.25ns 1906.9±47.09ns 100%
Expression (Parser) 4.9±0.12µs 4.9±0.14µs 100%
Fibonacci (Execution) 2.8±0.06ms 3.0±0.05ms 90.99999999999999%
For loop (Execution) 442.2±14.81µs 479.5±16.33µs 92%
For loop (Lexer) 5.1±0.11µs 5.1±0.14µs 99%
For loop (Parser) 13.8±0.54µs 13.5±0.31µs 102%
Hello World (Lexer) 897.5±23.92ns 921.9±32.51ns 97%
Hello World (Parser) 2.2±0.13µs 2.2±0.07µs 100%
Symbols (Execution) 436.7±9.22µs 481.2±9.01µs 89.99999999999999%
undefined undefined 100%

@HalidOdat HalidOdat added this to the v0.8.0 milestone May 9, 2020
@github-actions
Copy link

github-actions bot commented May 9, 2020

Benchmark for 06d578c

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 420.8±47.41µs 479.5±34.65µs 86.00000000000001%
Expression (Lexer) 2.1±0.13µs 1850.5±191.26ns 113.99999999999999%
Expression (Parser) 5.1±0.37µs 5.2±0.27µs 96%
Fibonacci (Execution) 3.2±0.19ms 3.4±0.27ms 92%
For loop (Execution) 475.1±26.81µs 533.5±16.98µs 87.99999999999999%
For loop (Lexer) 5.1±0.66µs 5.5±0.34µs 89.99999999999999%
For loop (Parser) 14.4±1.44µs 14.6±1.04µs 99%
Hello World (Lexer) 980.4±75.73ns 951.5±74.15ns 103%
Hello World (Parser) 2.5±0.16µs 2.4±0.12µs 101%
Symbols (Execution) 482.1±25.76µs 536.9±37.31µs 88.99999999999999%
undefined undefined 100%

@Razican
Copy link
Member

Razican commented May 9, 2020

Wow, performance gains, especially in execution, are impressive!

@HalidOdat
Copy link
Member Author

Wow, performance gains, especially in execution, are impressive!

I think it's because I removed a lot of unnecessary cloning

@HalidOdat HalidOdat marked this pull request as ready for review May 9, 2020 08:39
@HalidOdat HalidOdat requested review from Razican and jasonwilliams May 9, 2020 08:39
@github-actions
Copy link

github-actions bot commented May 9, 2020

Benchmark for 6507b17

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 404.7±15.16µs 473.3±27.01µs 83%
Expression (Lexer) 1943.2±89.79ns 1974.5±339.24ns 98%
Expression (Parser) 5.0±0.28µs 4.9±0.23µs 101%
Fibonacci (Execution) 2.8±0.12ms 3.1±0.15ms 89.99999999999999%
For loop (Execution) 430.6±27.66µs 490.6±27.88µs 86.00000000000001%
For loop (Lexer) 5.0±0.22µs 5.2±0.28µs 96%
For loop (Parser) 13.9±0.89µs 13.8±0.77µs 100%
Hello World (Lexer) 911.0±68.42ns 895.7±40.35ns 102%
Hello World (Parser) 2.1±0.14µs 2.3±0.08µs 93%
Symbols (Execution) 452.5±22.25µs 491.9±36.03µs 90.99999999999999%
undefined undefined 100%

@HalidOdat
Copy link
Member Author

HalidOdat commented May 9, 2020

I think this is ready to merge now :)

I plan to do more cleanup/refactoring of builtin methods in another PR.

Copy link
Member

@Razican Razican left a comment

Choose a reason for hiding this comment

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

This looks very clean and understandable. Performance has also improved a lot, which is nice. Give a look to my comments, just in case :)

boa/src/builtins/value/mod.rs Show resolved Hide resolved
boa/src/builtins/value/mod.rs Outdated Show resolved Hide resolved
@Razican Razican added performance Performance related changes and issues technical debt labels May 9, 2020
@github-actions
Copy link

github-actions bot commented May 9, 2020

Benchmark for 7d91994

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 358.7±19.88µs 408.7±28.62µs 86.00000000000001%
Expression (Lexer) 1726.0±100.74ns 1686.1±106.22ns 102%
Expression (Parser) 4.2±0.36µs 4.2±0.37µs 101%
Fibonacci (Execution) 2.8±0.15ms 2.9±0.14ms 96%
For loop (Execution) 399.0±31.36µs 439.5±32.18µs 89.99999999999999%
For loop (Lexer) 4.6±0.31µs 4.5±0.33µs 101%
For loop (Parser) 12.6±0.98µs 11.6±0.68µs 108%
Hello World (Lexer) 820.2±86.58ns 819.7±74.90ns 100%
Hello World (Parser) 2.0±0.14µs 1968.6±149.07ns 104%
Symbols (Execution) 394.9±32.04µs 428.3±37.93µs 92%
undefined undefined 100%

@github-actions
Copy link

github-actions bot commented May 9, 2020

Benchmark for 13464bc

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 402.3±11.32µs 457.4±33.56µs 86.00000000000001%
Expression (Lexer) 1863.6±78.01ns 1843.2±45.82ns 101%
Expression (Parser) 4.8±0.13µs 4.8±0.14µs 101%
Fibonacci (Execution) 2.8±0.06ms 3.0±0.10ms 89.99999999999999%
For loop (Execution) 425.6±11.75µs 473.0±16.78µs 88.99999999999999%
For loop (Lexer) 5.1±0.20µs 5.0±0.14µs 100%
For loop (Parser) 13.4±0.54µs 13.5±0.41µs 100%
Hello World (Lexer) 895.5±29.97ns 899.9±23.94ns 100%
Hello World (Parser) 2.2±0.07µs 2.2±0.05µs 99%
Symbols (Execution) 434.8±23.22µs 479.1±13.37µs 89.99999999999999%
undefined undefined 100%

Co-authored-by: Iban Eguia <[email protected]>
@github-actions
Copy link

github-actions bot commented May 9, 2020

Benchmark for 5392ac1

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 447.6±22.29µs 486.3±7.99µs 90.99999999999999%
Expression (Lexer) 2.0±0.03µs 2.1±0.05µs 99%
Expression (Parser) 5.1±0.13µs 5.1±0.11µs 100%
Fibonacci (Execution) 3.3±0.04ms 3.6±0.05ms 89.99999999999999%
For loop (Execution) 466.5±12.92µs 520.5±13.80µs 87.99999999999999%
For loop (Lexer) 5.5±0.17µs 5.5±0.10µs 99%
For loop (Parser) 15.0±0.25µs 15.0±0.28µs 100%
Hello World (Lexer) 985.7±34.85ns 994.8±18.05ns 99%
Hello World (Parser) 2.4±0.07µs 2.4±0.06µs 99%
Symbols (Execution) 473.9±6.88µs 523.6±9.12µs 88.99999999999999%
undefined undefined 100%

@github-actions
Copy link

github-actions bot commented May 9, 2020

Benchmark for bfe4aa1

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 416.4±16.40µs 409.5±32.77µs 102%
Expression (Lexer) 1807.1±147.55ns 2.0±0.08µs 87.00000000000001%
Expression (Parser) 4.7±0.40µs 4.8±0.31µs 97%
Fibonacci (Execution) 2.8±0.19ms 3.4±0.12ms 81%
For loop (Execution) 437.9±26.38µs 442.4±46.21µs 99%
For loop (Lexer) 5.1±0.43µs 4.9±0.40µs 103%
For loop (Parser) 14.0±0.90µs 14.2±1.10µs 99%
Hello World (Lexer) 881.1±83.87ns 972.8±72.84ns 89.99999999999999%
Hello World (Parser) 2.2±0.25µs 2.2±0.17µs 102%
Symbols (Execution) 423.8±29.83µs 432.6±47.65µs 98%
undefined undefined 100%

Copy link
Member

@Razican Razican left a comment

Choose a reason for hiding this comment

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

This is good to go!

boa/src/builtins/value/mod.rs Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented May 9, 2020

Benchmark for 1fab45b

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 396.5±11.40µs 431.6±15.14µs 90.99999999999999%
Expression (Lexer) 1882.7±52.20ns 1869.9±48.96ns 101%
Expression (Parser) 5.1±0.51µs 5.0±0.20µs 102%
Fibonacci (Execution) 2.7±0.08ms 3.0±0.20ms 88.99999999999999%
For loop (Execution) 408.3±23.18µs 446.1±15.86µs 90.99999999999999%
For loop (Lexer) 5.2±0.13µs 5.2±0.20µs 100%
For loop (Parser) 13.7±0.49µs 13.8±0.27µs 99%
Hello World (Lexer) 908.2±22.03ns 918.3±27.18ns 99%
Hello World (Parser) 2.2±0.09µs 2.2±0.11µs 102%
Symbols (Execution) 418.6±18.63µs 453.2±33.30µs 92%
undefined undefined 100%

@HalidOdat HalidOdat merged commit 1e18cb0 into master May 9, 2020
@HalidOdat HalidOdat deleted the refactor/value branch May 9, 2020 14:06
@HalidOdat HalidOdat mentioned this pull request May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance related changes and issues technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants