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.

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

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

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

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

randall77 opened this issue Aug 5, 2017 · 4 comments
Labels
FrozenDueToAge Performance
Milestone
Go1.10

Comments

@randall77
Copy link
Contributor

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
Copy link
Contributor

martisch commented Aug 6, 2017 •
edited

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
Copy link
Contributor

@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
Copy link
Member

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
Copy link

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

@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.
Assignees
No one assigned
Labels
FrozenDueToAge Performance
Projects
None yet
Milestone
Go1.10
Development

No branches or pull requests
5 participants
@josharian @martisch @aclements @randall77 @gopherbot