Skip to content

draft: implement bufferToString #45562

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
Closed
kiliczsh wants to merge 9 commits into from
Closed

draft: implement bufferToString #45562

kiliczsh wants to merge 9 commits into from

Conversation

@kiliczsh
Copy link

@kiliczsh kiliczsh commented Nov 21, 2022

Trying to implement suggested performance improvement in nodejs/performance#16.

@nodejs-github-bot nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Nov 21, 2022
@kiliczsh kiliczsh changed the title src: implement bufferToString draft: implement bufferToString Nov 21, 2022
anonrig
anonrig reviewed Nov 21, 2022
View reviewed changes
@mscdex
Copy link
Contributor

mscdex commented Nov 21, 2022

Did you see the linked comment in the code comment for this function? I think we shouldn't be spending effort on optimizing APIs that exist solely for compatibility with older code.

@anonrig
Copy link
Member

anonrig commented Nov 21, 2022

Did you see the linked comment in the code comment for this function? I think we shouldn't be spending effort on optimizing APIs that exist solely for compatibility with older code.

@mscdex This change also applies to several other places like _http_client.js and random.js (Referencing nodejs/performance#16)

@mscdex
Copy link
Contributor

mscdex commented Nov 21, 2022

This change also applies to several other places like _http_client.js and random.js

That's fine, I'm referring specifically to btoa() (and atob() for that matter), which is what this PR is currently changing.

anonrig
anonrig reviewed Nov 21, 2022
View reviewed changes
@kiliczsh kiliczsh requested a review from anonrig November 24, 2022 16:31
@kiliczsh
Copy link
Author

kiliczsh commented Jan 16, 2023

It is not applicable anymore due to nodejs/performance#16

@kiliczsh kiliczsh closed this Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anonrig anonrig Awaiting requested review from anonrig

Assignees

No one assigned

Labels

buffer Issues and PRs related to the buffer subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kiliczsh @mscdex @anonrig @nodejs-github-bot