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

Added jemallocator only for Linux targets #312

Merged
merged 1 commit into from
Apr 18, 2020
Merged

Added jemallocator only for Linux targets #312

merged 1 commit into from
Apr 18, 2020

Conversation

Razican
Copy link
Member

@Razican Razican commented Apr 13, 2020

As per the list of supported targets for jemallocator, it seems that only x86_64-unknown-linux-gnu is properly supported with all tests passing. So I added a conditional dependency that will only build in those targets and will only use jemallocator then. This fixes #309.

@Razican Razican added bug Something isn't working performance Performance related changes and issues labels Apr 13, 2020
@github-actions
Copy link

Benchmark for aec9f79

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 407.7±21.28µs 398.6±16.81µs 102%
Expression (Parser) 2.9±0.15µs 3.0±0.18µs 97%
Hello World (Execution) 437.1±22.61µs 444.0±33.57µs 98%
Hello World (Lexer) 1026.0±64.13ns 933.4±52.80ns 110.00000000000001%
Hello World (Parser) 1138.4±55.96ns 1065.1±58.09ns 107%
Symbol Creation 512.3±30.05µs 528.8±26.76µs 97%
fibonacci (Execution) 4.8±0.17ms 4.7±0.21ms 101%
undefined undefined 100%

@github-actions
Copy link

Benchmark for 1a9db43

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 458.5±24.46µs 453.1±24.25µs 101%
Expression (Parser) 3.4±0.21µs 3.4±0.22µs 100%
Hello World (Execution) 484.9±15.73µs 487.4±60.85µs 100%
Hello World (Lexer) 1034.7±49.06ns 1140.2±38.56ns 89.99999999999999%
Hello World (Parser) 1218.3±85.42ns 1243.6±72.74ns 98%
Symbol Creation 587.5±40.43µs 558.2±31.79µs 105%
fibonacci (Execution) 5.3±0.18ms 5.2±0.21ms 100%
undefined undefined 100%

@Razican
Copy link
Member Author

Razican commented Apr 13, 2020

Interestingly enough, benchmarks don't seem to be improving that much. Maybe it's not using jemallocator? I will check it in my computer.

@jasonwilliams
Copy link
Member

This now builds fine on Windows

@jasonwilliams
Copy link
Member

Interestingly enough, benchmarks don't seem to be improving that much. Maybe it's not using jemallocator? I will check it in my computer.

I'm not sure, maybe the triple isn't quite right?

@Razican
Copy link
Member Author

Razican commented Apr 13, 2020

Should be fixed now. It turns out there is no target in cfg, as per the reference, and we need to specify each part separately. Let's wait for the benchmarks.

@github-actions
Copy link

Benchmark for fddf015

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 325.5±23.17µs 380.2±24.33µs 83%
Expression (Parser) 2.6±0.24µs 2.9±0.22µs 86.00000000000001%
Hello World (Execution) 346.9±28.41µs 405.2±24.60µs 83%
Hello World (Lexer) 1046.9±154.52ns 863.7±51.76ns 121%
Hello World (Parser) 1098.0±87.26ns 967.7±53.91ns 112.99999999999999%
Symbol Creation 354.6±24.05µs 479.1±32.31µs 64.99999999999999%
fibonacci (Execution) 3.8±0.15ms 4.6±0.29ms 78%
undefined undefined 100%

@github-actions
Copy link

Benchmark for 62c90e0

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 424.7±23.54µs 495.1±93.87µs 83%
Expression (Parser) 3.1±0.09µs 3.7±0.40µs 82%
Hello World (Execution) 424.2±15.39µs 491.9±19.30µs 84.00000000000001%
Hello World (Lexer) 1140.3±40.36ns 1188.6±48.79ns 96%
Hello World (Parser) 1353.0±50.97ns 1245.3±44.53ns 109.00000000000001%
Symbol Creation 469.4±28.16µs 612.3±38.99µs 70%
fibonacci (Execution) 4.6±0.10ms 5.4±0.14ms 82%
undefined undefined 100%

@Razican
Copy link
Member Author

Razican commented Apr 13, 2020

I guess that the lexer/parser for hello world show like this because of the build infrastructure, they are consistently better in my machine (at least the lexer). This is ready for a merge, in any case.

We should probably add Windows as a testing target, though, so that we can catch these faster.

@HalidOdat
Copy link
Member

HalidOdat commented Apr 14, 2020

We should probably add Windows as a testing target, though, so that we can catch these faster.

I would like to add this. It shouldn't be hard with github actions.

@Razican
Copy link
Member Author

Razican commented Apr 14, 2020

I would like to add this. It shouldn't be hard with github actions.

Sure, let's do it in a different PR :) do you want to work on it?

@HalidOdat
Copy link
Member

HalidOdat commented Apr 14, 2020

Sure, let's do it in a different PR :) do you want to work on it?

Yes. I'm really interested in github Actions.

@Razican
Copy link
Member Author

Razican commented Apr 15, 2020

This is ready for a merge :)

@jasonwilliams
Copy link
Member

I’ll give it another test

@HalidOdat HalidOdat added the linux Issues and PRs related to the Linux platform label Apr 16, 2020
@github-actions
Copy link

Benchmark for 5c1bdd6

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 355.0±11.97µs 446.1±26.96µs 74%
Expression (Lexer) 2.2±0.13µs 2.2±0.17µs 104%
Expression (Parser) 5.0±0.19µs 4.8±0.23µs 105%
Fibonacci (Execution) 4.0±0.15ms 5.1±0.12ms 74%
For loop (Execution) 377.6±13.69µs 535.1±15.93µs 58.00000000000001%
For loop (Lexer) 5.3±0.97µs 5.2±0.25µs 102%
For loop (Parser) 13.2±0.38µs 15.5±0.52µs 82%
Hello World (Lexer) 944.9±53.12ns 1012.6±40.26ns 93%
Hello World (Parser) 2.1±0.06µs 2.2±0.19µs 96%
Symbols (Execution) 386.9±13.68µs 552.4±29.46µs 57.00000000000001%
undefined undefined 100%

@github-actions
Copy link

Benchmark for d6394a6

Click to view benchmark
Test PR Benchmark Master Benchmark %
Create Realm 310.8±16.26µs 368.8±11.75µs 81%
Expression (Lexer) 1898.1±59.73ns 1791.4±67.37ns 106%
Expression (Parser) 4.4±0.22µs 4.1±0.12µs 108%
Fibonacci (Execution) 3.3±0.13ms 4.1±0.13ms 74%
For loop (Execution) 314.5±13.06µs 452.9±13.82µs 56.00000000000001%
For loop (Lexer) 4.7±0.19µs 5.1±0.26µs 89.99999999999999%
For loop (Parser) 11.1±0.49µs 12.8±0.44µs 85.00000000000001%
Hello World (Lexer) 802.8±33.29ns 883.5±36.57ns 89.99999999999999%
Hello World (Parser) 1849.7±84.95ns 1884.1±101.27ns 98%
Symbols (Execution) 334.6±12.68µs 464.3±17.51µs 61.00000000000001%
undefined undefined 100%

@Razican
Copy link
Member Author

Razican commented Apr 17, 2020

I rebased, and it seems that the performance gains are very nice, especially on execution :)

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 perfect to me! 👍

@Razican Razican requested a review from jasonwilliams April 17, 2020 14:33
@Razican
Copy link
Member Author

Razican commented Apr 17, 2020

Cool, let's wait for @jasonwilliams to finish the tests :)

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.

works fine on Windows 10

@jasonwilliams jasonwilliams added this to the v0.8.0 milestone Apr 18, 2020
@HalidOdat HalidOdat merged commit 44de190 into master Apr 18, 2020
@HalidOdat HalidOdat deleted the fix_309 branch April 18, 2020 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working linux Issues and PRs related to the Linux platform performance Performance related changes and issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jemalloc is not building on windows
3 participants