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

Refine r2/kv detach buffer flag to avoid copy #313

Closed
wants to merge 1 commit into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Jan 25, 2023

Concerns over the possible performance hit of copying were raised. This adjusts the flag to preserve the post-call modification behavior when the detach flag is not set. Also sets the flag as explicitly experimental so we can experiment with it before releasing.

Concerns over the possible performance hit of copying were raised.
This adjusts the flag to preserve the post-call modification behavior
when the detach flag is not set. Also sets the flag as explicitly
experimental so we can experiment with it before releasing.
@jasnell jasnell closed this Feb 28, 2023
@a-robinson
Copy link
Member

Hm, so did we just never fix the underlying concern here? I'm trying to find any sort of resolution but all I see is:

I don't see any clear conclusion either here or anywhere I could find internally. Do you remember where this ended up, @jasnell ?

@kentonv
Copy link
Member

kentonv commented Jun 15, 2023

@a-robinson I believe nothing ever happened. There's some disagreement over whether we should do anything -- the existing behavior is technically wrong but we have no evidence that anyone has ever been impacted, whereas "fixing" it would involve a performance hit, at least in the absence of some sort of copy-on-write feature being added to V8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants