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.

Sign up for GitHub

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

Jump to bottom

Feature BigInt #358

Merged
merged 6 commits into from
May 19, 2020
Merged

Feature BigInt #358

merged 6 commits into from
May 19, 2020

Conversation

HalidOdat
Copy link
Member

@HalidOdat HalidOdat commented May 1, 2020 •
edited
Loading

This PR resolves #121

@github-actions
Copy link

github-actions bot commented May 1, 2020

Benchmark for 903cdcf

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 369.9±9.62µs 373.3±18.26µs 99%
Expression (Lexer) 1894.5±37.68ns 1884.0±49.65ns 101%
Expression (Parser) 4.8±0.15µs 4.9±0.14µs 97%
Fibonacci (Execution) 3.5±0.09ms 3.5±0.10ms 99%
For loop (Execution) 373.0±9.69µs 378.7±9.60µs 98%
For loop (Lexer) 5.0±0.20µs 5.1±0.17µs 99%
For loop (Parser) 13.9±0.64µs 13.8±0.32µs 101%
Hello World (Lexer) 889.8±29.49ns 873.5±27.52ns 102%
Hello World (Parser) 2.2±0.05µs 2.2±0.08µs 98%
Symbols (Execution) 386.4±11.23µs 385.3±20.24µs 100%
undefined undefined 100%

@Razican Razican added the enhancement New feature or request label May 1, 2020
@Razican
Copy link
Member

Razican commented May 7, 2020

Hey, how is this going? can we help?

@HalidOdat
Copy link
Member Author

Hey, how is this going? can we help?

The reason I didn't update this PR is because I was preoccupied with refactoring. I wanted to refactor things before contributing to the mess. (Already have some progress locally)

Speaking of help, I have a question about which crate should use.
The best candidates are currently rug and num-bigint.
Pros of rug:

  • It's very optimized, (internally it uses gmp-sys)
  • It's got every thing I need for implementing BigInt.

Cons of rug:

  • It has and dependency on an c library (slower compile-time)
  • On windows it requires msys2 to be preinstalled as a compilation dependency.

Pros of num-bigint:

  • It requires no c dependencies.
  • Faster compilation times.

Cons of num-bigint:

  • It's not the fastest implementation (they don't claim either)
  • requires num-trait crate to get some needed functionality.

What should I use? what is the best choice? @Razican

@Razican
Copy link
Member

Razican commented May 7, 2020

Hey, how is this going? can we help?

The reason I didn't update this PR is because I was preoccupied with refactoring. I wanted to refactor things before contributing to the mess. (Already have some progress locally)

This sounds very reasonable, thanks a lot for that!

Speaking of help, I have a question about which crate should use.
The best candidates are currently rug and num-bigint.
Pros of rug:

* It's very **optimized**, (internally it uses `gmp-sys`)

* It's got every thing I need for implementing BigInt.

Cons of rug:

* It has and dependency on an c library (slower compile-time)

* On windows it requires `msys2` to be preinstalled as a compilation dependency.

Pros of num-bigint:

* It requires no c dependencies.

* Faster compilation times.

Cons of num-bigint:

* It's not the fastest implementation (they don't claim either)

* requires `num-trait` crate to get some needed functionality.

What should I use? what is the best choice? @Razican

I think we should go for num-bigint. One of the things we are trying to develop is a pure-Rust engine, even if it's not currently 100% optimised. Maybe in the future we can contribute to num-bigint to improve performance, what do you think, @jasonwilliams ?

About using extra crates, I think it's ok, we only use it because we really need it, and dependencies are small enough I think so that they won't add so much burden on compile times.

@HalidOdat
Copy link
Member Author

HalidOdat commented May 7, 2020 •
edited
Loading

I think we should go for num-bigint. One of the things we are trying to develop is a pure-Rust engine, even if it's not currently 100% optimised. Maybe in the future we can contribute to num-bigint to improve performance, what do you think, @jasonwilliams ?

About using extra crates, I think it's ok, we only use it because we really need it, and dependencies are small enough I think so that they won't add so much burden on compile times.

Thanks for the feedback that really helps.

This was referenced May 8, 2020
Merged
Closed
@HalidOdat
Copy link
Member Author

While trying to implement this, I ran into a problem with the abstract equality, so this is blocked until #395 lands.

@Razican Razican added the blocked Waiting for another code change label May 12, 2020
@Razican Razican mentioned this pull request May 13, 2020
Merged
@HalidOdat HalidOdat removed the blocked Waiting for another code change label May 13, 2020
@HalidOdat HalidOdat mentioned this pull request May 14, 2020
Closed
@github-actions
Copy link

Benchmark for bee902c

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 452.3±21.79µs 443.1±20.17µs 102%
Expression (Lexer) 2.2±0.09µs 2.2±0.16µs 99%
Expression (Parser) 5.5±0.40µs 5.4±0.47µs 102%
Fibonacci (Execution) 3.4±0.14ms 3.3±0.10ms 104%
For loop (Execution) 470.1±18.57µs 479.8±29.90µs 98%
For loop (Lexer) 5.9±0.34µs 5.7±0.24µs 103%
For loop (Parser) 16.5±1.03µs 16.2±0.89µs 102%
Hello World (Lexer) 1097.9±98.12ns 1094.3±93.40ns 100%
Hello World (Parser) 2.6±0.17µs 2.7±0.22µs 99%
Symbols (Execution) 484.5±20.15µs 475.3±14.45µs 102%
undefined undefined 100%

@jasonwilliams
Copy link
Member

I think we should go for num-bigint. One of the things we are trying to develop is a pure-Rust engine, even if it's not currently 100% optimised. Maybe in the future we can contribute to num-bigint to improve performance, what do you think, @jasonwilliams ?

agree 100%

@github-actions
Copy link

Benchmark for 84a9c5b

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 417.1±18.97µs 407.3±24.19µs 102%
Expression (Lexer) 2.1±0.10µs 1957.4±113.24ns 106%
Expression (Parser) 5.1±0.27µs 5.1±0.31µs 100%
Fibonacci (Execution) 3.0±0.11ms 3.0±0.12ms 102%
For loop (Execution) 449.5±29.95µs 437.5±23.41µs 103%
For loop (Lexer) 5.1±0.23µs 5.2±0.31µs 98%
For loop (Parser) 15.0±0.83µs 14.7±0.94µs 102%
Hello World (Lexer) 982.8±55.08ns 1001.9±42.20ns 98%
Hello World (Parser) 2.4±0.13µs 2.4±0.13µs 102%
Symbols (Execution) 449.3±22.76µs 443.1±21.80µs 101%
undefined undefined 100%

@github-actions
Copy link

Benchmark for 4e9390e

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 412.7±16.32µs 417.2±220.20µs 99%
Expression (Lexer) 1966.7±80.03ns 1925.8±95.67ns 102%
Expression (Parser) 4.8±0.21µs 4.9±0.20µs 98%
Fibonacci (Execution) 2.9±0.14ms 2.9±0.10ms 101%
For loop (Execution) 431.5±29.33µs 418.7±15.10µs 103%
For loop (Lexer) 5.1±0.22µs 5.1±0.35µs 100%
For loop (Parser) 14.5±0.82µs 14.0±0.51µs 104%
Hello World (Lexer) 955.8±39.50ns 966.7±42.85ns 99%
Hello World (Parser) 2.3±0.10µs 2.3±0.10µs 101%
Symbols (Execution) 442.4±19.61µs 429.9±17.81µs 103%
undefined undefined 100%

@github-actions
Copy link

Benchmark for 4188177

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 431.4±26.66µs 398.5±33.20µs 108%
Expression (Lexer) 2.1±0.07µs 1891.1±119.45ns 110.00000000000001%
Expression (Parser) 5.0±0.19µs 4.7±0.22µs 107%
Fibonacci (Execution) 3.0±0.19ms 3.0±0.10ms 103%
For loop (Execution) 456.7±27.42µs 421.1±20.91µs 108%
For loop (Lexer) 5.4±0.27µs 5.0±0.32µs 110.00000000000001%
For loop (Parser) 14.9±0.49µs 14.4±0.59µs 104%
Hello World (Lexer) 1022.4±85.05ns 1030.9±88.02ns 99%
Hello World (Parser) 2.5±0.20µs 2.4±0.08µs 106%
Symbols (Execution) 499.1±57.44µs 430.3±20.10µs 115.99999999999999%
undefined undefined 100%

@github-actions
Copy link

Benchmark for 867ed91

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 431.0±13.66µs 415.1±7.85µs 104%
Expression (Lexer) 2.0±0.07µs 2.0±0.19µs 100%
Expression (Parser) 5.3±0.08µs 5.2±0.19µs 103%
Fibonacci (Execution) 2.9±0.05ms 2.9±0.04ms 99%
For loop (Execution) 441.0±9.05µs 436.4±8.96µs 101%
For loop (Lexer) 5.4±0.16µs 5.4±0.10µs 101%
For loop (Parser) 14.9±0.26µs 14.5±0.29µs 103%
Hello World (Lexer) 988.3±21.99ns 1000.4±27.24ns 99%
Hello World (Parser) 2.5±0.06µs 2.4±0.07µs 102%
Symbols (Execution) 449.7±6.34µs 452.9±14.28µs 99%
undefined undefined 100%

@github-actions
Copy link

Benchmark for 385194a

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 442.2±63.17µs 419.2±53.76µs 105%
Expression (Lexer) 1964.0±188.74ns 2.0±0.19µs 98%
Expression (Parser) 4.9±0.47µs 5.0±0.59µs 97%
Fibonacci (Execution) 3.1±0.45ms 3.2±0.31ms 99%
For loop (Execution) 457.7±54.04µs 445.6±49.93µs 103%
For loop (Lexer) 5.5±0.37µs 5.6±0.72µs 97%
For loop (Parser) 14.5±1.42µs 14.7±1.25µs 99%
Hello World (Lexer) 955.3±93.71ns 1062.1±85.48ns 88.99999999999999%
Hello World (Parser) 2.6±0.22µs 2.6±0.23µs 102%
Symbols (Execution) 460.8±52.01µs 468.8±38.63µs 98%
undefined undefined 100%

@HalidOdat HalidOdat marked this pull request as ready for review May 15, 2020 08:15
@github-actions
Copy link

Benchmark for 93fd798

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 417.0±24.98µs 404.7±16.43µs 103%
Expression (Lexer) 1955.7±25.23ns 1900.8±21.53ns 103%
Expression (Parser) 5.2±0.06µs 5.0±0.30µs 105%
Fibonacci (Execution) 2.8±0.10ms 2.8±0.09ms 99%
For loop (Execution) 441.2±26.13µs 424.6±20.24µs 104%
For loop (Lexer) 5.2±0.29µs 5.3±0.26µs 99%
For loop (Parser) 14.4±0.43µs 13.9±0.36µs 103%
Hello World (Lexer) 932.6±20.03ns 970.6±28.55ns 96%
Hello World (Parser) 2.4±0.13µs 2.3±0.04µs 105%
Symbols (Execution) 443.1±20.37µs 426.4±23.30µs 104%
undefined undefined 100%

@github-actions
Copy link

Benchmark for 00ab819

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 413.6±15.73µs 404.0±26.89µs 102%
Expression (Lexer) 1959.9±80.10ns 1953.2±106.58ns 100%
Expression (Parser) 5.1±0.14µs 5.0±0.28µs 102%
Fibonacci (Execution) 2.8±0.06ms 2.8±0.07ms 99%
For loop (Execution) 435.1±27.10µs 420.9±14.52µs 103%
For loop (Lexer) 5.2±0.17µs 5.6±0.35µs 94%
For loop (Parser) 14.5±0.54µs 14.5±1.80µs 100%
Hello World (Lexer) 957.3±24.73ns 1000.4±36.65ns 95%
Hello World (Parser) 2.4±0.10µs 2.3±0.09µs 104%
Symbols (Execution) 445.6±17.75µs 437.9±55.59µs 102%
undefined undefined 100%

Razican
Razican approved these changes May 16, 2020
View reviewed changes
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 good :) check my comments, but I think it's almost ready.

boa/src/builtins/bigint/mod.rs Outdated Show resolved Hide resolved
boa/src/builtins/value/tests.rs Outdated Show resolved Hide resolved
boa/src/exec/mod.rs Outdated Show resolved Hide resolved
@HalidOdat HalidOdat force-pushed the feature/bigint branch 3 times, most recently from d13e6eb to ec12eba Compare May 17, 2020 04:25
@github-actions
Copy link

Benchmark for e16c9ca

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 381.9±17.04µs 378.3±14.20µs 101%
Expression (Lexer) 1914.1±72.87ns 1881.6±61.92ns 102%
Expression (Parser) 4.7±0.14µs 4.8±0.16µs 96%
Fibonacci (Execution) 2.7±0.09ms 2.7±0.13ms 100%
For loop (Execution) 412.7±14.23µs 407.8±16.69µs 101%
For loop (Lexer) 4.9±0.17µs 5.1±0.20µs 97%
For loop (Parser) 13.7±0.54µs 13.7±0.44µs 101%
Hello World (Lexer) 922.6±24.53ns 952.8±30.83ns 97%
Hello World (Parser) 2.3±0.06µs 2.3±0.08µs 99%
Symbols (Execution) 421.2±13.66µs 404.7±16.59µs 104%
undefined undefined 100%

@github-actions
Copy link

Benchmark for 36dffae

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 329.3±21.17µs 326.4±27.49µs 101%
Expression (Lexer) 1661.7±111.47ns 1579.1±104.38ns 105%
Expression (Parser) 4.3±0.27µs 4.0±0.27µs 105%
Fibonacci (Execution) 2.3±0.12ms 2.2±0.09ms 102%
For loop (Execution) 356.8±24.38µs 337.6±22.48µs 106%
For loop (Lexer) 4.3±0.24µs 4.3±0.30µs 101%
For loop (Parser) 11.8±0.75µs 11.6±0.73µs 102%
Hello World (Lexer) 783.8±51.43ns 795.8±52.65ns 98%
Hello World (Parser) 1960.4±145.75ns 1901.5±127.63ns 103%
Symbols (Execution) 352.9±24.95µs 357.8±37.25µs 99%
undefined undefined 100%

@github-actions
Copy link

Benchmark for 4e1fafd

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 321.2±25.98µs 318.7±22.84µs 101%
Expression (Lexer) 1777.4±83.68ns 1640.4±144.87ns 108%
Expression (Parser) 4.6±0.24µs 4.2±0.32µs 108%
Fibonacci (Execution) 2.3±0.23ms 2.4±0.18ms 98%
For loop (Execution) 356.0±39.74µs 379.3±24.04µs 93%
For loop (Lexer) 4.5±0.24µs 5.0±0.20µs 87.99999999999999%
For loop (Parser) 12.7±0.65µs 11.6±1.16µs 109.00000000000001%
Hello World (Lexer) 836.4±40.91ns 868.7±53.70ns 96%
Hello World (Parser) 2.2±0.07µs 1874.7±146.00ns 117%
Symbols (Execution) 356.0±32.14µs 375.9±26.72µs 94%
undefined undefined 100%

@github-actions
Copy link

Benchmark for 71a582d

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 380.7±11.84µs 387.7±8.03µs 98%
Expression (Lexer) 1903.6±45.00ns 1750.5±67.59ns 109.00000000000001%
Expression (Parser) 4.9±0.15µs 4.7±0.21µs 104%
Fibonacci (Execution) 2.6±0.07ms 2.6±0.06ms 98%
For loop (Execution) 399.5±23.30µs 410.3±30.65µs 97%
For loop (Lexer) 5.1±0.15µs 4.7±0.20µs 108%
For loop (Parser) 14.0±0.20µs 13.4±0.29µs 105%
Hello World (Lexer) 912.7±23.16ns 915.9±34.59ns 100%
Hello World (Parser) 2.4±0.05µs 2.2±0.08µs 108%
Symbols (Execution) 415.4±13.91µs 418.7±19.21µs 99%
undefined undefined 100%

@github-actions
Copy link

Benchmark for 4030270

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 358.2±27.20µs 339.2±23.52µs 106%
Expression (Lexer) 1630.9±164.21ns 1774.9±96.39ns 90.99999999999999%
Expression (Parser) 4.5±0.44µs 4.7±0.20µs 96%
Fibonacci (Execution) 2.4±0.16ms 2.5±0.09ms 96%
For loop (Execution) 366.4±33.77µs 376.0±23.56µs 97%
For loop (Lexer) 4.1±0.33µs 4.9±0.31µs 79%
For loop (Parser) 13.2±1.06µs 13.6±0.65µs 97%
Hello World (Lexer) 816.8±95.97ns 906.5±39.03ns 88.99999999999999%
Hello World (Parser) 2.3±0.19µs 2.3±0.11µs 99%
Symbols (Execution) 382.5±28.98µs 368.0±30.26µs 104%
undefined undefined 100%

@github-actions
Copy link

Benchmark for 67bafe0

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 411.7±21.85µs 395.1±25.06µs 104%
Expression (Lexer) 1970.6±108.70ns 1929.0±87.37ns 102%
Expression (Parser) 5.0±0.29µs 4.8±0.30µs 103%
Fibonacci (Execution) 2.8±0.11ms 2.8±0.12ms 101%
For loop (Execution) 425.4±22.52µs 413.1±22.27µs 103%
For loop (Lexer) 5.1±0.30µs 5.2±0.29µs 98%
For loop (Parser) 14.8±0.62µs 14.3±0.79µs 103%
Hello World (Lexer) 976.4±70.12ns 991.3±61.90ns 98%
Hello World (Parser) 2.4±0.12µs 2.3±0.12µs 105%
Symbols (Execution) 439.4±19.55µs 424.0±95.23µs 104%
undefined undefined 100%

Razican
Razican reviewed May 18, 2020
View reviewed changes
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.

Looks very good, check my comments just in case :)

boa/src/builtins/value/operations.rs Outdated Show resolved Hide resolved
boa/src/syntax/ast/bigint.rs Outdated Show resolved Hide resolved
boa/src/syntax/ast/constant.rs Show resolved Hide resolved
@github-actions
Copy link

Benchmark for ed9ba9e

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 317.9±16.65µs 319.4±16.87µs 100%
Expression (Lexer) 1606.8±125.38ns 1529.9±93.89ns 105%
Expression (Parser) 4.3±0.28µs 4.1±0.33µs 105%
Fibonacci (Execution) 2.2±0.11ms 2.1±0.12ms 101%
For loop (Execution) 341.0±20.74µs 331.0±21.71µs 103%
For loop (Lexer) 4.2±0.30µs 4.1±0.25µs 103%
For loop (Parser) 11.4±0.80µs 12.8±1.01µs 87.99999999999999%
Hello World (Lexer) 787.7±64.41ns 774.2±46.69ns 102%
Hello World (Parser) 1896.6±156.01ns 1957.1±132.42ns 97%
Symbols (Execution) 340.8±23.36µs 338.6±18.62µs 101%
undefined undefined 100%

@github-actions
Copy link

Benchmark for 0945f5e

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 382.5±9.99µs 378.1±12.05µs 101%
Expression (Lexer) 1909.1±83.78ns 1868.8±47.17ns 102%
Expression (Parser) 5.0±0.27µs 4.8±0.13µs 105%
Fibonacci (Execution) 2.6±0.08ms 2.6±0.07ms 101%
For loop (Execution) 409.5±16.50µs 395.0±13.84µs 104%
For loop (Lexer) 5.2±0.12µs 4.9±0.16µs 105%
For loop (Parser) 14.0±0.55µs 13.6±0.72µs 103%
Hello World (Lexer) 921.2±27.47ns 925.8±43.69ns 99%
Hello World (Parser) 2.3±0.07µs 2.3±0.07µs 102%
Symbols (Execution) 413.7±14.92µs 402.5±14.54µs 103%
undefined undefined 100%

@github-actions
Copy link

Benchmark for 93cbc19

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 379.5±18.15µs 396.0±24.62µs 96%
Expression (Lexer) 1919.7±73.53ns 1805.5±155.22ns 106%
Expression (Parser) 5.0±0.37µs 4.9±0.12µs 101%
Fibonacci (Execution) 2.7±0.08ms 2.5±0.11ms 107%
For loop (Execution) 419.9±17.06µs 391.0±18.66µs 107%
For loop (Lexer) 4.9±0.16µs 5.2±0.12µs 95%
For loop (Parser) 14.3±0.65µs 13.9±0.30µs 103%
Hello World (Lexer) 916.2±27.45ns 947.6±31.41ns 97%
Hello World (Parser) 2.3±0.08µs 2.3±0.09µs 101%
Symbols (Execution) 415.0±6.93µs 404.3±21.19µs 103%
undefined undefined 100%

@github-actions
Copy link

Benchmark for d828bc1

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 359.1±16.48µs 359.2±14.89µs 100%
Expression (Lexer) 1681.2±72.63ns 1738.2±82.13ns 97%
Expression (Parser) 4.5±0.17µs 4.5±0.19µs 99%
Fibonacci (Execution) 2.3±0.09ms 2.4±0.07ms 97%
For loop (Execution) 365.0±16.98µs 383.7±17.54µs 95%
For loop (Lexer) 4.6±0.26µs 4.8±0.22µs 95%
For loop (Parser) 12.7±0.59µs 13.4±0.74µs 95%
Hello World (Lexer) 854.4±35.54ns 886.5±36.23ns 96%
Hello World (Parser) 2.1±0.09µs 2.1±0.07µs 98%
Symbols (Execution) 382.1±14.31µs 380.5±13.94µs 100%
undefined undefined 100%

Razican
Razican approved these changes May 18, 2020
View reviewed changes
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 for me :)

jasonwilliams
jasonwilliams approved these changes May 19, 2020
View reviewed changes
Copy link
Member

@jasonwilliams jasonwilliams left a comment

Choose a reason for hiding this comment

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

Great work @HalidOdat !

@HalidOdat HalidOdat merged commit 5f4a1f2 into master May 19, 2020
@HalidOdat HalidOdat deleted the feature/bigint branch May 19, 2020 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Manishearth Manishearth Manishearth left review comments

@jasonwilliams jasonwilliams jasonwilliams approved these changes

@Razican Razican Razican approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
v0.8.0
Development

Successfully merging this pull request may close these issues.

[Feature] - BigInt
4 participants
@HalidOdat @Razican @jasonwilliams @Manishearth