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

Prevent unnecessary safecopy in iterator parseKV #971

Merged
merged 2 commits into from
Aug 10, 2019

Conversation

jarifibrahim
Copy link
Contributor

@jarifibrahim jarifibrahim commented Aug 7, 2019

The previous implementation of parseKV would copy over the value slice
every time, which is not necessary. The value slice should be copied only
when we return the result of point lookups. The proposed patch remove
safecopy call from the iterator.


This change is Reviewable

The previous implementation of parseKV would copy over the value slice
every time, which is not necessary. The value slice should be copied only
when we return the result of point lookups. The proposed patch remove
safecopy call from the iterator.
@jarifibrahim jarifibrahim requested a review from a team August 7, 2019 14:03
Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

✅ A review job has been created and sent to the PullRequest network.


@jarifibrahim you can click here to see the review status or cancel the code review job.

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm: Please test this thoroughly.

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ashish-goswami)

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

✖️ This code review was cancelled. See Details

@jarifibrahim
Copy link
Contributor Author

@ashish-goswami Review, please.

@ashish-goswami
Copy link
Contributor

Just check why travis is failing.

@jarifibrahim
Copy link
Contributor Author

The build on mac OS is failing on travis. It is failing for all PRs.

@jarifibrahim jarifibrahim merged commit 74ed6da into master Aug 10, 2019
@jarifibrahim jarifibrahim deleted the ibrahim/prevent-safecopy branch August 10, 2019 11:05
jarifibrahim pushed a commit that referenced this pull request Mar 12, 2020
The previous implementation of parseKV would copy over the value slice
every time we iterate, which is not necessary. The value slice should be
copied only when we return the result of point lookups. The proposed patch
removes safeCopy call from the iterator.

(cherry picked from commit 74ed6da)
jarifibrahim pushed a commit that referenced this pull request Mar 13, 2020
This reverts commit 4a22cac.

This branch does not contain the other block iterator changes. This
patch causes segmentation fault.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants