hash/maphash: Provide a purego implementation

[flimzy]

What version of Go are you using (go version)?

1.17rc1

What operating system and processor architecture are you using (go env)?

GopherJS with Go 1.17

What did you do?

64d323f improves the performance of the hash/maphash package, but in doing so makes the package even "less" safe than it was before. In 1.16 this was worked around in GopherJS (where pointer arithmetic is not possible) with gopherjs/gopherjs@ccebfda. The recent changes will require a much larger delta, as now every caller to rthash will need to be updated, to use calling semantics more similar to the 1.16 version.

What did you expect hope to see?

A purego implementation of hash/maphash would solve this problem. And perhaps it would be useful in other scenarios as well?

[flimzy]

cc @dmitshur

[mvdan]

My initial reaction was "I thought purego was for avoiding asm and cgo, not unsafe", but I'm wrong: #23172 (comment)

[randall77]

Are you proposing to replace hash/maphash with a purego implementation? What implementation would that be?
unsafe is used in that package for performance reasons. Any different implementation would probably need to be equally fast.

[flimzy]

Are you proposing to replace hash/maphash with a purego implementation?

No. I would keep the existing one for performance where useable, and add a purego version behind build tags.

What implementation would that be?

TBD. But probably logically equivalent to what GopherJS is doing now (i.e. a slight variation from the Go 1.16 version).

[randall77]

So far the only purego tags in the main repo are crypto libraries. I think there's a higher level decision to be made as to how much we want to sprinkle the standard library with purego backstops. For instance, we'd probably need a purego builder to test that the code works.
But other than that (big) concern, it sounds reasonable to me.

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rsc]

In net and os/user we have netgo and osusergo tags instead of purego.
The only purego I see is in some crypto packages.
However, it looks like we accepted the idea of purego and did not get around to the documentation of it: #23172.

Given that, it seems fine to have a purego hash/maphash.

[cristaloleg]

Not sure but maybe this should be accepted first? #23172

[randall77]

#23172 is accepted. It just needs to be done.

[rsc]

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

[rsc]

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

[nevkontakte]

I would like to contribute to the implementation, but I don't think I would be able to help much with testing infrastructure concerns @randall77 mentioned in #47342 (comment).

One alternative I can think of is that both optimized and purego implementations are included in the default build as private functions, and build constraints only select which one is actually used. In that case it would be possible to add a test that verifies that both implementation behave exactly the same. Dead code elimination should be able to strip out one of the implementations in the build.

The drawback of this option would be that the code might get a bit less intuitive, and it's not an approach that's easy to generalize other packages we might want a purego version of.

[gopherbot]

Change https://go.dev/cl/468795 mentions this issue: hash/maphash: add purego implementation

[shehackedyou]

This ultimately will make a novice programmers try to use binaries from system commands; and instead will be able to build proper libraries around ABIs from C libraries; this project may have a specific scope for you but overall this is pretty massive for the Go community; it will make it both easier and faster to use C tools which was becoming a problem.