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

Speed up Array.prototype.join() #1635

Open
wants to merge 4 commits into
base: static_h
Choose a base branch
from

Conversation

tmikov
Copy link
Contributor

@tmikov tmikov commented Feb 28, 2025

Summary:
The improvement is based on the following ideas:

  • Optimistically assume the input array already contains strings and
    avoid copying them into a temporary array.
  • Switch to the slower path for the remaining elements, once a
    non-string is encountered.
  • Use faster access if the input is a dense array.

Apparently this call is used by print(), so it is tested a lot
implicitly, for example by the array-push.js test.

Differential Revision: D70415235

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Feb 28, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D70415235

Summary:
The last commit added more tests, but the output that was compared
wasn't precise. Improve the verbosity of the output.

Differential Revision: D70370734
Summary: Improve the tests of `toReversed()` and fix the bug that was revealed.

Differential Revision: D70370910
Summary:
Add a fast path when the input is a dense array.
Speed up the slow path:
- Move SmallHermesValue-s around to avoid decoding and encoding
  when possible.
- Only flush the GCScope when needed.
- Efficiently store into the output array, since we know it is an array.

The case of a non-dense input JSArray should be fairly fast compared to
before. Have I benchmarked it? No.

Differential Revision: D70372051
tmikov pushed a commit to tmikov/hermes that referenced this pull request Mar 1, 2025
Summary:

The improvement is based on the following ideas:
- Optimistically assume the input array already contains strings and
avoid copying them into a temporary array.
- Switch to the slower path for the remaining elements, once a
non-string is encountered.
- Use faster access if the input is a dense array.

Apparently this call is used by `print()`, so it is tested a lot
implicitly, for example by the `array-push.js` test.

Reviewed By: avp

Differential Revision: D70415235
@tmikov tmikov force-pushed the export-D70415235-to-static_h branch from c60074b to 8d916b4 Compare March 1, 2025 02:21
Summary:
Pull Request resolved: facebook#1635

The improvement is based on the following ideas:
- Optimistically assume the input array already contains strings and
avoid copying them into a temporary array.
- Switch to the slower path for the remaining elements, once a
non-string is encountered.
- Use faster access if the input is a dense array.

Apparently this call is used by `print()`, so it is tested a lot
implicitly, for example by the `array-push.js` test.

Reviewed By: avp

Differential Revision: D70415235
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D70415235

@tmikov tmikov force-pushed the export-D70415235-to-static_h branch from 8d916b4 to 93f6f39 Compare March 1, 2025 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants