Skip to content

Conversation

@ExE-Boss
Copy link
Contributor

Refs: #36304 (comment)
Refs: #36565

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@Lxxyx
Copy link
Member

Lxxyx commented Dec 22, 2020

Run Benchmark on my computer, seems the performance issues still exists.

                           confidence improvement accuracy (*)    (**)   (***)
 misc/freelist.js n=100000        ***    -92.46 %      ±1.94% ±2.61% ±3.46%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case there are 1 comparisons, you can thus
expect the following amount of false-positive results:
  0.05 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.01 false positives, when considering a   1% risk acceptance (**, ***),
  0.00 false positives, when considering a 0.1% risk acceptance (***)

@aduh95
Copy link
Contributor

aduh95 commented Dec 22, 2020

@ExE-Boss
Copy link
Contributor Author

Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

Benchmark confirmed very poor performance results…

                            confidence improvement accuracy (*)   (**)  (***)
 misc/freelist.js n=100000        ***    -91.36 %       ±3.89% ±5.24% ±6.96%

@targos targos added the lib / src Issues and PRs related to general changes in the lib or src directory. label Dec 27, 2020
@ExE-Boss
Copy link
Contributor Author

ExE-Boss commented Mar 7, 2021

If I had to guess, I’d say V8 is doing some inlining shenanigans when it detects a push or pop method on an ordinary array with %Array.prototype% in its [[Prototype]] internal slot.


By inlining the methods in #37649, I hope to make FreeList faster and resilient to mutation of %Array.prototype%.pop and %Array.prototype%.push

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs related to general changes in the lib or src directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants