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

Move Object internal object methods to GcObject #835

Merged
merged 1 commit into from
Oct 16, 2020
Merged

Move Object internal object methods to GcObject #835

merged 1 commit into from
Oct 16, 2020

Conversation

HalidOdat
Copy link
Member

@HalidOdat HalidOdat commented Oct 9, 2020 •
edited
Loading

Why move the methods?
Moving Object internal methods to GcObject because we borrow the object and we hold on to it and if an accessor (getters/setters) was to access the object it will give a borrow panic.

I expect some performance regression, since we do more borrow checks.

@HalidOdat HalidOdat added bug Something isn't working technical debt execution Issues or PRs related to code execution labels Oct 9, 2020
@HalidOdat HalidOdat added this to the v0.11.0 milestone Oct 9, 2020
@codecov
Copy link

codecov bot commented Oct 9, 2020 •
edited
Loading

Codecov Report

Merging #835 into master will decrease coverage by 0.01%.
The diff coverage is 53.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #835      +/-   ##
==========================================
- Coverage   59.30%   59.29%   -0.02%     
==========================================
  Files         165      165              
  Lines       10342    10376      +34     
==========================================
+ Hits         6133     6152      +19     
- Misses       4209     4224      +15
Impacted Files Coverage Δ
boa/src/builtins/array/array_iterator.rs 77.55% <ø> (ø)
boa/src/builtins/iterable/mod.rs 94.23% <ø> (ø)
boa/src/builtins/map/map_iterator.rs 71.42% <ø> (ø)
boa/src/builtins/string/string_iterator.rs 77.50% <ø> (ø)
boa/src/class.rs 0.00% <ø> (ø)
boa/src/context.rs 61.64% <0.00%> (+0.55%) ⬆️
boa/src/environment/global_environment_record.rs 27.95% <ø> (ø)
boa/src/environment/object_environment_record.rs 23.07% <ø> (ø)
boa/src/object/mod.rs 46.45% <0.00%> (-0.54%) ⬇️
boa/src/builtins/object/mod.rs 64.00% <38.09%> (-0.87%) ⬇️
... and 10 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 dc1628a...25fe301. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Oct 9, 2020

Benchmark for 44ad89a

Click to view benchmark
Test PR Benchmark Master Benchmark %
Arithmetic operations (Execution) 393.2±26.46ns 383.4±23.46ns +2.56%
Arithmetic operations (Full) 252.8±16.51µs 249.4±13.28µs +1.36%
Array access (Execution) 8.4±0.54µs 8.3±0.49µs +1.20%
Array access (Full) 283.5±18.80µs 288.4±22.96µs -1.70%
Array creation (Execution) 2.8±0.12ms 2.9±0.17ms -3.45%
Array creation (Full) 3.1±0.19ms 3.0±0.12ms +3.33%
Array pop (Execution) 1053.6±56.65µs 1029.2±51.06µs +2.37%
Array pop (Full) 1540.8±108.07µs 1461.6±94.07µs +5.42%
Boolean Object Access (Execution) 4.7±0.21µs 4.6±0.30µs +2.17%
Boolean Object Access (Full) 276.8±14.31µs 266.4±14.32µs +3.90%
Clean js (Execution) 749.3±34.81µs 725.1±41.57µs +3.34%
Clean js (Full) 1069.5±59.75µs 992.6±53.72µs +7.75%
Clean js (Parser) 38.8±4.66µs 41.1±3.71µs -5.60%
Create Realm 473.9±27.10ns 492.7±32.13ns -3.82%
Dynamic Object Property Access (Execution) 6.1±0.44µs 6.3±0.42µs -3.17%
Dynamic Object Property Access (Full) 285.6±15.76µs 270.3±13.22µs +5.66%
Expression (Parser) 7.2±0.46µs 7.2±0.61µs 0.00%
Fibonacci (Execution) 934.1±48.67µs 947.1±56.93µs -1.37%
Fibonacci (Full) 1296.9±64.98µs 1241.0±50.88µs +4.50%
For loop (Execution) 24.2±2.53µs 24.0±2.19µs +0.83%
For loop (Full) 307.7±12.23µs 307.7±14.32µs 0.00%
For loop (Parser) 19.4±1.09µs 19.3±0.87µs +0.52%
Goal Symbols (Parser) 12.8±0.77µs 13.2±1.23µs -3.03%
Hello World (Parser) 3.4±0.18µs 3.3±0.16µs +3.03%
Long file (Parser) 791.2±40.73ns 813.4±40.13ns -2.73%
Mini js (Execution) 676.6±30.13µs 644.6±37.61µs +4.96%
Mini js (Full) 952.8±109.13µs 914.2±42.25µs +4.22%
Mini js (Parser) 35.3±3.89µs 34.0±2.42µs +3.82%
Number Object Access (Execution) 3.8±0.21µs 3.8±0.28µs 0.00%
Number Object Access (Full) 266.9±15.31µs 261.1±11.78µs +2.22%
Object Creation (Execution) 5.3±0.31µs 5.3±0.31µs 0.00%
Object Creation (Full) 276.0±15.26µs 259.1±12.47µs +6.52%
RegExp (Execution) 10.0±0.67µs 10.0±0.47µs 0.00%
RegExp (Full) 275.3±29.92µs 274.2±14.35µs +0.40%
RegExp Literal (Execution) 11.4±1.22µs 10.9±0.52µs +4.59%
RegExp Literal (Full) 272.7±14.50µs 293.3±13.40µs -7.02%
RegExp Literal Creation (Execution) 10.0±0.68µs 9.9±0.50µs +1.01%
RegExp Literal Creation (Full) 270.8±15.75µs 268.4±18.31µs +0.89%
Static Object Property Access (Execution) 5.5±0.34µs 5.4±0.31µs +1.85%
Static Object Property Access (Full) 275.7±14.02µs 279.2±13.96µs -1.25%
String Object Access (Execution) 7.3±0.31µs 7.3±0.71µs 0.00%
String Object Access (Full) 282.8±13.52µs 266.7±19.19µs +6.04%
String comparison (Execution) 7.4±0.58µs 6.5±0.50µs +13.85%
String comparison (Full) 264.4±16.69µs 279.0±13.21µs -5.23%
String concatenation (Execution) 5.8±0.31µs 5.3±0.39µs +9.43%
String concatenation (Full) 271.6±14.66µs 268.8±20.80µs +1.04%
String copy (Execution) 4.5±0.30µs 4.4±0.44µs +2.27%
String copy (Full) 269.4±11.81µs 264.3±13.90µs +1.93%
Symbols (Execution) 3.8±0.27µs 3.8±0.33µs 0.00%
Symbols (Full) 256.1±16.92µs 242.8±12.92µs +5.48%

@HalidOdat
Copy link
Member Author

HalidOdat commented Oct 10, 2020 •
edited
Loading

Looking at the benchmarks there isn't that big of a regression, as I expected.

@Razican
Copy link
Member

Razican commented Oct 10, 2020

Looking at the benchmarks there isn't that big of a regression, as I expected.

I'm guessing many times the compiler can statically remove some checks.

@HalidOdat HalidOdat force-pushed the move-object-methods branch from fd16259 to 5109d83 Compare October 10, 2020 11:27
@HalidOdat HalidOdat marked this pull request as ready for review October 10, 2020 11:29
@HalidOdat HalidOdat mentioned this pull request Oct 10, 2020
Closed
@github-actions
Copy link

Benchmark for a4e0864

Click to view benchmark
Test PR Benchmark Master Benchmark %
Arithmetic operations (Execution) 375.5±26.15ns 375.5±21.17ns 0.00%
Arithmetic operations (Full) 250.6±16.40µs 257.2±15.00µs -2.57%
Array access (Execution) 9.0±0.55µs 8.8±0.49µs +2.27%
Array access (Full) 275.5±9.89µs 275.5±13.48µs 0.00%
Array creation (Execution) 2.8±0.22ms 3.1±0.19ms -9.68%
Array creation (Full) 3.3±0.17ms 3.0±0.09ms +10.00%
Array pop (Execution) 991.6±76.47µs 1062.6±37.59µs -6.68%
Array pop (Full) 1567.2±81.03µs 1565.8±122.73µs +0.09%
Boolean Object Access (Execution) 4.9±0.33µs 4.8±0.22µs +2.08%
Boolean Object Access (Full) 268.0±15.41µs 277.1±17.96µs -3.28%
Clean js (Execution) 787.8±56.53µs 751.1±44.15µs +4.89%
Clean js (Full) 1061.8±57.93µs 1056.4±76.51µs +0.51%
Clean js (Parser) 41.2±3.29µs 40.5±2.63µs +1.73%
Create Realm 489.1±37.71ns 485.4±21.48ns +0.76%
Dynamic Object Property Access (Execution) 6.2±0.35µs 6.1±0.27µs +1.64%
Dynamic Object Property Access (Full) 273.2±16.44µs 279.9±16.32µs -2.39%
Expression (Parser) 7.3±0.44µs 7.6±0.51µs -3.95%
Fibonacci (Execution) 948.8±54.60µs 927.6±36.82µs +2.29%
Fibonacci (Full) 1229.5±63.10µs 1239.7±76.17µs -0.82%
For loop (Execution) 24.5±1.45µs 25.5±1.77µs -3.92%
For loop (Full) 302.6±19.88µs 304.5±18.35µs -0.62%
For loop (Parser) 19.6±1.29µs 19.5±1.08µs +0.51%
Goal Symbols (Parser) 13.2±0.82µs 13.4±0.81µs -1.49%
Hello World (Parser) 3.5±0.25µs 3.5±0.17µs 0.00%
Long file (Parser) 880.9±68.94ns 831.2±39.72ns +5.98%
Mini js (Execution) 673.9±35.85µs 666.6±35.84µs +1.10%
Mini js (Full) 957.0±43.04µs 951.3±48.10µs +0.60%
Mini js (Parser) 37.3±2.77µs 35.7±2.05µs +4.48%
Number Object Access (Execution) 3.8±0.16µs 3.9±0.22µs -2.56%
Number Object Access (Full) 270.6±13.34µs 268.4±16.60µs +0.82%
Object Creation (Execution) 5.1±0.46µs 5.2±0.26µs -1.92%
Object Creation (Full) 270.0±17.59µs 290.0±19.74µs -6.90%
RegExp (Execution) 10.2±0.68µs 10.7±0.65µs -4.67%
RegExp (Full) 288.9±20.23µs 286.9±18.25µs +0.70%
RegExp Literal (Execution) 11.2±0.70µs 11.7±0.80µs -4.27%
RegExp Literal (Full) 285.9±20.47µs 287.4±18.72µs -0.52%
RegExp Literal Creation (Execution) 10.2±0.74µs 10.3±1.10µs -0.97%
RegExp Literal Creation (Full) 269.1±14.62µs 284.8±16.20µs -5.51%
Static Object Property Access (Execution) 5.3±0.36µs 5.5±0.29µs -3.64%
Static Object Property Access (Full) 280.7±15.69µs 268.6±16.21µs +4.50%
String Object Access (Execution) 7.6±0.45µs 7.4±0.34µs +2.70%
String Object Access (Full) 275.5±17.44µs 279.5±20.25µs -1.43%
String comparison (Execution) 7.1±0.47µs 7.1±0.28µs 0.00%
String comparison (Full) 275.9±19.21µs 274.7±19.37µs +0.44%
String concatenation (Execution) 5.8±0.47µs 5.7±0.24µs +1.75%
String concatenation (Full) 261.1±15.03µs 272.7±15.01µs -4.25%
String copy (Execution) 4.5±0.27µs 4.6±0.28µs -2.17%
String copy (Full) 264.3±15.52µs 267.2±21.49µs -1.09%
Symbols (Execution) 3.8±0.27µs 3.6±0.18µs +5.56%
Symbols (Full) 247.3±14.96µs 246.8±13.11µs +0.20%

RageKnify
RageKnify approved these changes Oct 10, 2020
View reviewed changes
Copy link
Contributor

@RageKnify RageKnify left a comment

Choose a reason for hiding this comment

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

Other than updating the comments and the lifetime naming it looks fine.

@github-actions
Copy link

Benchmark for 35865cf

Click to view benchmark
Test PR Benchmark Master Benchmark %
Arithmetic operations (Execution) 361.1±1.86ns 360.3±4.15ns +0.22%
Arithmetic operations (Full) 231.2±3.05µs 228.4±2.59µs +1.23%
Array access (Execution) 7.8±0.18µs 7.7±0.14µs +1.30%
Array access (Full) 258.4±19.83µs 245.4±3.70µs +5.30%
Array creation (Execution) 2.8±0.07ms 2.9±0.03ms -3.45%
Array creation (Full) 3.1±0.04ms 3.0±0.03ms +3.33%
Array pop (Execution) 1064.9±17.74µs 1048.3±18.91µs +1.58%
Array pop (Full) 1473.3±18.01µs 1455.6±15.40µs +1.22%
Boolean Object Access (Execution) 4.6±0.03µs 4.5±0.05µs +2.22%
Boolean Object Access (Full) 240.6±1.83µs 240.3±3.06µs +0.12%
Clean js (Execution) 719.1±4.73µs 688.2±8.97µs +4.49%
Clean js (Full) 1011.4±10.68µs 979.0±9.23µs +3.31%
Clean js (Parser) 37.0±0.07µs 36.6±0.62µs +1.09%
Create Realm 457.6±14.96ns 461.2±6.05ns -0.78%
Dynamic Object Property Access (Execution) 5.7±0.04µs 5.5±0.06µs +3.64%
Dynamic Object Property Access (Full) 247.3±1.26µs 243.6±4.02µs +1.52%
Expression (Parser) 6.9±0.06µs 6.8±0.11µs +1.47%
Fibonacci (Execution) 821.7±14.39µs 792.0±10.60µs +3.75%
Fibonacci (Full) 1111.9±3.51µs 1082.0±9.80µs +2.76%
For loop (Execution) 22.5±0.11µs 22.2±0.29µs +1.35%
For loop (Full) 268.4±1.88µs 267.8±3.52µs +0.22%
For loop (Parser) 17.8±0.15µs 17.7±0.24µs +0.56%
Goal Symbols (Parser) 12.2±0.16µs 12.4±0.12µs -1.61%
Hello World (Parser) 3.1±0.01µs 3.0±0.06µs +3.33%
Long file (Parser) 758.6±4.36ns 746.9±13.25ns +1.57%
Mini js (Execution) 645.6±5.93µs 612.4±12.48µs +5.42%
Mini js (Full) 914.0±4.53µs 883.4±8.18µs +3.46%
Mini js (Parser) 32.2±0.21µs 32.0±0.46µs +0.63%
Number Object Access (Execution) 3.7±0.04µs 3.6±0.04µs +2.78%
Number Object Access (Full) 238.4±1.33µs 238.8±2.52µs -0.17%
Object Creation (Execution) 4.8±0.05µs 4.7±0.06µs +2.13%
Object Creation (Full) 238.2±2.87µs 241.0±2.56µs -1.16%
RegExp (Execution) 9.2±0.14µs 9.2±0.10µs 0.00%
RegExp (Full) 250.5±0.78µs 247.8±3.42µs +1.09%
RegExp Literal (Execution) 10.4±0.17µs 10.1±0.11µs +2.97%
RegExp Literal (Full) 250.6±2.94µs 248.9±4.05µs +0.68%
RegExp Literal Creation (Execution) 9.4±0.28µs 9.1±0.10µs +3.30%
RegExp Literal Creation (Full) 244.1±1.98µs 239.9±5.03µs +1.75%
Static Object Property Access (Execution) 5.0±0.02µs 4.9±0.05µs +2.04%
Static Object Property Access (Full) 241.9±1.88µs 237.1±4.00µs +2.02%
String Object Access (Execution) 7.0±0.09µs 6.8±0.06µs +2.94%
String Object Access (Full) 245.9±1.40µs 243.6±2.11µs +0.94%
String comparison (Execution) 6.5±0.11µs 6.3±0.07µs +3.17%
String comparison (Full) 245.9±3.44µs 246.2±3.23µs -0.12%
String concatenation (Execution) 5.3±0.09µs 5.2±0.08µs +1.92%
String concatenation (Full) 242.7±1.03µs 241.6±1.60µs +0.46%
String copy (Execution) 4.1±0.03µs 3.9±0.04µs +5.13%
String copy (Full) 235.1±0.66µs 230.5±2.46µs +2.00%
Symbols (Execution) 3.5±0.04µs 3.3±0.05µs +6.06%
Symbols (Full) 223.5±2.56µs 220.3±2.63µs +1.45%

@HalidOdat HalidOdat force-pushed the move-object-methods branch from cb5d69f to 25fe301 Compare October 16, 2020 17:51
@github-actions
Copy link

Benchmark for 55881f0

Click to view benchmark
Test PR Benchmark Master Benchmark %
Arithmetic operations (Execution) 374.1±10.18ns 363.9±0.25ns +2.80%
Arithmetic operations (Full) 246.4±6.95µs 250.3±10.41µs -1.56%
Array access (Execution) 8.2±0.04µs 8.0±0.45µs +2.50%
Array access (Full) 269.2±11.11µs 274.7±10.67µs -2.00%
Array creation (Execution) 2.9±0.00ms 3.0±0.10ms -3.33%
Array creation (Full) 3.1±0.00ms 3.2±0.18ms -3.13%
Array pop (Execution) 1062.8±3.85µs 1057.1±3.66µs +0.54%
Array pop (Full) 1491.7±2.28µs 1555.8±55.00µs -4.12%
Boolean Object Access (Execution) 4.7±0.01µs 4.6±0.02µs +2.17%
Boolean Object Access (Full) 249.6±6.33µs 264.6±14.91µs -5.67%
Clean js (Execution) 724.5±3.89µs 703.7±3.85µs +2.96%
Clean js (Full) 1020.7±8.93µs 1047.8±60.41µs -2.59%
Clean js (Parser) 38.2±2.53µs 38.3±1.88µs -0.26%
Create Realm 480.5±18.17ns 496.7±31.35ns -3.26%
Dynamic Object Property Access (Execution) 5.8±0.27µs 5.8±0.27µs 0.00%
Dynamic Object Property Access (Full) 253.7±2.17µs 258.0±0.58µs -1.67%
Expression (Parser) 7.1±0.26µs 7.1±0.52µs 0.00%
Fibonacci (Execution) 860.8±24.64µs 866.1±96.59µs -0.61%
Fibonacci (Full) 1150.7±41.97µs 1125.4±33.36µs +2.25%
For loop (Execution) 23.4±1.27µs 23.3±0.93µs +0.43%
For loop (Full) 281.0±5.79µs 288.5±6.33µs -2.60%
For loop (Parser) 18.5±0.03µs 18.4±0.02µs +0.54%
Goal Symbols (Parser) 12.6±0.02µs 13.0±0.38µs -3.08%
Hello World (Parser) 3.4±0.00µs 3.5±0.11µs -2.86%
Long file (Parser) 763.9±16.21ns 768.5±0.82ns -0.60%
Mini js (Execution) 652.4±6.10µs 634.9±4.09µs +2.76%
Mini js (Full) 916.0±4.75µs 955.3±59.32µs -4.11%
Mini js (Parser) 32.4±0.04µs 32.4±0.19µs 0.00%
Number Object Access (Execution) 3.7±0.01µs 3.7±0.01µs 0.00%
Number Object Access (Full) 255.0±11.66µs 261.0±10.96µs -2.30%
Object Creation (Execution) 5.0±0.17µs 4.9±0.21µs +2.04%
Object Creation (Full) 258.0±9.92µs 254.0±0.86µs +1.57%
RegExp (Execution) 9.7±0.41µs 9.4±0.28µs +3.19%
RegExp (Full) 255.6±0.54µs 269.2±11.53µs -5.05%
RegExp Literal (Execution) 11.0±1.11µs 10.6±0.32µs +3.77%
RegExp Literal (Full) 258.6±1.17µs 275.1±18.63µs -6.00%
RegExp Literal Creation (Execution) 9.8±0.62µs 9.5±0.41µs +3.16%
RegExp Literal Creation (Full) 252.3±0.46µs 267.1±8.29µs -5.54%
Static Object Property Access (Execution) 5.4±0.36µs 5.3±0.98µs +1.89%
Static Object Property Access (Full) 262.1±12.91µs 262.0±11.30µs +0.04%
String Object Access (Execution) 7.1±0.12µs 7.0±0.02µs +1.43%
String Object Access (Full) 259.0±6.55µs 270.4±10.91µs -4.22%
String comparison (Execution) 6.7±0.34µs 6.9±0.67µs -2.90%
String comparison (Full) 254.7±0.57µs 259.7±0.36µs -1.93%
String concatenation (Execution) 5.3±0.03µs 5.3±0.35µs 0.00%
String concatenation (Full) 247.9±0.29µs 253.3±0.32µs -2.13%
String copy (Execution) 4.1±0.03µs 4.1±0.12µs 0.00%
String copy (Full) 253.7±22.63µs 248.7±0.44µs +2.01%
Symbols (Execution) 3.5±0.09µs 3.4±0.01µs +2.94%
Symbols (Full) 243.0±6.77µs 243.5±0.37µs -0.21%

@HalidOdat HalidOdat merged commit 2cb2442 into master Oct 16, 2020
@HalidOdat HalidOdat deleted the move-object-methods branch October 16, 2020 18:28
@HalidOdat HalidOdat mentioned this pull request Jul 28, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RageKnify RageKnify RageKnify approved these changes

@jasonwilliams jasonwilliams Awaiting requested review from jasonwilliams

@Razican Razican Awaiting requested review from Razican

@Lan2u Lan2u Awaiting requested review from Lan2u

Assignees
No one assigned
Labels
bug Something isn't working execution Issues or PRs related to code execution technical debt
Projects
None yet
Milestone
v0.11.0
Development

Successfully merging this pull request may close these issues.
3 participants
@HalidOdat @Razican @RageKnify