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

LibJS/Bytecode: Better codegen for this #24504

Merged
merged 6 commits into from
Jun 1, 2024
Merged

LibJS/Bytecode: Better codegen for this #24504

merged 6 commits into from
Jun 1, 2024

Conversation

awesomekling
Copy link
Contributor

@awesomekling awesomekling commented Jun 1, 2024 •
edited
Loading

Three main things here:

  • The cached this register is now pre-populated when entering a bytecode executable.
  • We no longer emit ResolveThisBinding at all when we know the pre-populated cache is non-empty (this is true for any function that doesn't require a unique function environment)
  • When we do have ResolveThisBinding, it no longer has a dst, but instead always just populates the cached this register.

Net result: fewer instructions! Same result! Go faster! Whee! (And also fixes a bug, included a test for that!)

Benchmark results:

Suite       Test                                   Speedup  Old (Mean ± Range)        New (Mean ± Range)
----------  -----------------------------------  ---------  ------------------------  ------------------------
Kraken      ai-astar.js                              1.1    2.470 ± 2.440 … 2.500     2.245 ± 2.200 … 2.290
Kraken      audio-beat-detection.js                  1.024  1.490 ± 1.450 … 1.530     1.455 ± 1.450 … 1.460
Kraken      audio-dft.js                             1.094  1.280 ± 1.270 … 1.290     1.170 ± 1.160 … 1.180
Kraken      audio-fft.js                             1.037  1.525 ± 1.470 … 1.580     1.470 ± 1.440 … 1.500
Kraken      audio-oscillator.js                      0.972  1.540 ± 1.540 … 1.540     1.585 ± 1.580 … 1.590
Kraken      imaging-darkroom.js                      1.005  2.050 ± 2.040 … 2.060     2.040 ± 2.040 … 2.040
Kraken      imaging-desaturate.js                    1.023  2.635 ± 2.630 … 2.640     2.575 ± 2.570 … 2.580
Kraken      imaging-gaussian-blur.js                 0.978  8.140 ± 7.920 … 8.360     8.325 ± 8.310 … 8.340
Kraken      json-parse-financial.js                  0.966  0.140 ± 0.140 … 0.140     0.145 ± 0.140 … 0.150
Kraken      json-stringify-tinderbox.js              0.885  0.230 ± 0.230 … 0.230     0.260 ± 0.260 … 0.260
Kraken      stanford-crypto-aes.js                   1.038  0.680 ± 0.670 … 0.690     0.655 ± 0.620 … 0.690
Kraken      stanford-crypto-ccm.js                   1.009  0.590 ± 0.590 … 0.590     0.585 ± 0.580 … 0.590
Kraken      stanford-crypto-pbkdf2.js                0.977  1.070 ± 1.070 … 1.070     1.095 ± 1.090 … 1.100
Kraken      stanford-crypto-sha256-iterative.js      0.989  0.435 ± 0.430 … 0.440     0.440 ± 0.420 … 0.460
Octane      box2d.js                                 0.949  2.135 ± 2.120 … 2.150     2.250 ± 2.060 … 2.440
Octane      code-load.js                             0.998  2.130 ± 2.130 … 2.130     2.135 ± 2.130 … 2.140
Octane      crypto.js                                0.922  6.315 ± 6.260 … 6.370     6.850 ± 6.320 … 7.380
Octane      deltablue.js                             0.998  2.015 ± 2.010 … 2.020     2.020 ± 2.020 … 2.020
Octane      earley-boyer.js                          0.984  15.560 ± 15.440 … 15.680  15.820 ± 14.980 … 16.660
Octane      gbemu.js                                 1.002  2.730 ± 2.630 … 2.830     2.725 ± 2.620 … 2.830
Octane      mandreel.js                              0.963  12.935 ± 12.890 … 12.980  13.430 ± 13.300 … 13.560
Octane      navier-stokes.js                         1.029  3.165 ± 3.160 … 3.170     3.075 ± 3.070 … 3.080
Octane      pdfjs.js                                 1.059  2.895 ± 2.810 … 2.980     2.735 ± 2.650 … 2.820
Octane      raytrace.js                              1.078  6.245 ± 6.220 … 6.270     5.795 ± 5.520 … 6.070
Octane      regexp.js                                1.064  23.035 ± 22.320 … 23.750  21.655 ± 21.560 … 21.750
Octane      richards.js                              1      2.010 ± 2.010 … 2.010     2.010 ± 2.010 … 2.010
Octane      splay.js                                 1.009  2.830 ± 2.800 … 2.860     2.805 ± 2.800 … 2.810
Octane      typescript.js                            1.023  26.485 ± 25.400 … 27.570  25.880 ± 25.280 … 26.480
Octane      zlib.js                                  1.049  78.190 ± 76.060 … 80.320  74.525 ± 74.410 … 74.640
Kraken      Total                                    1.01   24.275                    24.045
Octane      Total                                    1.027  188.675                   183.710
All Suites  Total                                    1.025  212.950                   207.755

@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Jun 1, 2024
awesomekling and others added 4 commits June 1, 2024 08:18
@awesomekling
LibJS/Bytecode: Always resolve this binding into dedicated register
fbd16c9 
We already have a dedicated register slot for `this`, so instead of
having ResolveThisBinding take a `dst` operand, just write the value
directly into the `this` register every time.
@awesomekling
LibJS/Bytecode: Pretty-print the this register as "this"
b679cde
@awesomekling
LibJS/Bytecode: Don't generate ResolveThisBinding if not needed
05fdfb9 
Functions that don't have a FunctionEnvironment will get their `this`
value from the ExecutionContext. This patch stops generating
ResolveThisBinding instructions at all for functions like that, and
instead pre-populates the `this` register when entering a new bytecode
executable.
@awesomekling @skyrising
LibJS: Add test to verify copying this to a local doesn't break
949005d 
Co-Authored-By: Simon Wanner <simon+git@skyrising.xyz>
@awesomekling awesomekling merged commit 8b7ad09 into SerenityOS:master Jun 1, 2024
11 checks passed
@github-actions github-actions bot removed the 👀 pr-needs-review PR needs review from a maintainer or community member label Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@awesomekling