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

Add RawKey function and make use of it. #56

Merged
merged 1 commit into from
Nov 17, 2016
Merged

Conversation

kevina
Copy link
Contributor

@kevina kevina commented Nov 16, 2016

Avoids some unnecessary creation of strings.

This decreased the time to retrieve 100,000 small (1000 byte) blocks (see ipfs/kubo#3376) from around 245 ms to 205 ms.

@kevina
Copy link
Contributor Author

kevina commented Nov 16, 2016

It looks like I broke something. Will fix.

Avoids some unnecessary creation of strings.
@kevina
Copy link
Contributor Author

kevina commented Nov 16, 2016

Should be fixed now.

@kevina
Copy link
Contributor Author

kevina commented Nov 16, 2016

@whyrusleeping should be good to go

@@ -29,8 +28,8 @@ func PrefixTransform(prefix ds.Key) ktds.KeyTransform {
panic("expected prefix not found")
}

s := strings.TrimPrefix(k.String(), prefix.String())
return ds.NewKey(s)
s := k.String()[len(prefix.String()):]
Copy link
Member

Choose a reason for hiding this comment

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

You should make sure to do a bounds check here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could but then what, panic. Also it is redundant because if prefix.IsAncestorOf passes then there is no way for this to be out of bounds.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, i see. I didnt realize we already had that expectation here.

@whyrusleeping whyrusleeping merged commit 971a83c into master Nov 17, 2016
@whyrusleeping whyrusleeping deleted the kevina/rawkey branch November 17, 2016 03:08
@whyrusleeping whyrusleeping removed the status/in-progress In progress label Nov 17, 2016
@whyrusleeping
Copy link
Member

LGTM, thanks!`

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.

2 participants