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

runtime: map{assign,delete}_fast routines should return ptr to key #21321

Closed
randall77 opened this issue Aug 5, 2017 · 4 comments

Comments

Projects
None yet
5 participants
@randall77
Copy link
Contributor

commented Aug 5, 2017

The _fast routines need to use typedmemove and typedmemclr to update the keys and values. Instead, we should return a pointer to the key and value and have the caller update them. That way, the caller can just do a direct write in the case where the key or value has no pointers.

... and maybe the non-fast versions also?

See #21297

@josharian @aclements

@randall77 randall77 added this to the Go1.10 milestone Aug 5, 2017

@josharian josharian self-assigned this Aug 6, 2017

@martisch

This comment has been minimized.

Copy link
Member

commented Aug 6, 2017

As an alternative could the map functions themselfs check if the type contains pointers and only use typedmemmove if writeBarrier.needed?
This would keep additional code outside the map functions small.

@josharian

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2017

@martisch I've had a change implementing that locally for a while. (IIRC, it led to me filing #20934.) I'll experiment a bit with alternatives here before sending anything.

@aclements

This comment has been minimized.

Copy link
Member

commented Aug 6, 2017

The reason I suspect returning the key pointer may be faster is because then the caller could use a more specialized write in the pointer case. For example, if it's a pointer key, the fast path would be just a few instructions. But it would probably be worth trying out both and comparing them.

@gopherbot

This comment has been minimized.

Copy link

commented Aug 25, 2017

Change https://golang.org/cl/59110 mentions this issue: runtime: optimize storing new keys in mapassign_fastNN

@gopherbot gopherbot closed this in 25bbb69 Aug 25, 2017

@golang golang locked and limited conversation to collaborators Aug 25, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.