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

GH-39248: [JS] Unify code paths for utf8 and largeUtf8 #39249

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

domoritz
Copy link
Member

@domoritz domoritz commented Dec 16, 2023

Reduce the code size by using common code paths. We only call Number a few times on numbers, which should be a noop.

@domoritz
Copy link
Member Author

@ursabot please benchmark lang=JavaScript

@ursabot
Copy link

ursabot commented Dec 16, 2023

Benchmark runs are scheduled for commit 4da251b. Watch https://buildkite.com/apache-arrow and https://conbench.ursa.dev for updates. A comment will be posted here when the runs are complete.

Copy link

Thanks for your patience. Conbench analyzed the 1 benchmarking run that has been run so far on PR commit 4da251b.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

@domoritz
Copy link
Member Author

Reviewed as part of #39258.

@domoritz domoritz merged commit 9c097d5 into apache:main Dec 18, 2023
11 checks passed
@domoritz domoritz deleted the largeUtf8-paths branch December 18, 2023 03:19
domoritz added a commit that referenced this pull request Dec 18, 2023
Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 9c097d5.

There were 2 benchmark results indicating a performance regression:

The full Conbench report has more details. It also includes information about 4 possible false positives for unstable benchmarks that are known to sometimes produce them.

clayburn pushed a commit to clayburn/arrow that referenced this pull request Jan 23, 2024
…39249)

Reduce the code size by using common code paths. We only call `Number` a
few times on numbers, which should be a noop.

* Closes: apache#39248
clayburn pushed a commit to clayburn/arrow that referenced this pull request Jan 23, 2024
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…39249)

Reduce the code size by using common code paths. We only call `Number` a
few times on numbers, which should be a noop.

* Closes: apache#39248
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[JS]: Unify code paths for utf8 and largeUtf8
2 participants