4 ms·

I was curious what function was used for hashing (how do you write a generic hash function in go?) and it's pretty disgusting. https://github.com/dolthub/mapha

I was curious what function was used for hashing (how do you write a generic hash function in go?) and it's pretty disgusting. https://github.com/dolthub/maphash/blob/main/runtime.go https://github.com/dolthub/maphash/blob/main/runtime.go

lukevp 3y ago
Could you provide more context on what’s disgusting? I’m not a go programmer but this looks like it’s just a wrapper around a library called ‘maphash’ and doesn’t do any of the hashing here? This is more of a service wrapper around that lib so that it can have a consistent seed value etc.?
- brabel 3y ago
  I suppose the use of unsafe pointers liberally is disgusting to some.
- sidlls 3y ago
  I wouldn't call it disgusting. It's just typical unsafe go code. The named return parameter in `getRuntimeHasher` is irritating, as all such parameters in go code are. If you're going to return values, do so explicitly in the return statement. In go code, seeing `return` at the end of a function does not mean that the function doesn't return any values, and that can lead to harder-to-read code.
  - insanitybit 3y ago
    > It's just typical unsafe go code. But... just to hash something?
    - sidlls 3y ago
      Go has a lot of lower level internal implementations of things that unsafe code like this serves as a thin shim for. It’s an almost zero-cost abstraction to use such a builtin. I’m not saying it’s good: it just is what it is. It’s silly: but so is most of go. I’ve worked with this language for several years now: I don’t like it at all. It presents a facade of simplicity, but has all sorts of hidden issues that affect performance and behavior in surprising ways.
      - insanitybit 3y ago
        I think we're all saying the same thing then.
- tedunangst 3y ago
  Constructing a map (an opaque type) just so you cast it to a carefully duplicated struct so you can steal the function pointer is kinda nutso. Did you look at the linked code in maphash?
oooyay 3y ago
Disgusting is a word, but I'm guessing you'd say the same about most unsupported feature code (aka unsafe). You can make code like this reliable in the ways it needs to be while still being "unsafe". That's kind of table stakes when you start dabbling in unsupported things.
- tedunangst 3y ago
  And what does this code do to make itself reliable in the event the go authors change the internal structure of a map?
  - rad_gruchalski 3y ago
    Nothing. That’s the trade off. What is not clear about “unsafe” in that context?
    - tedunangst 3y ago
      The part about making it reliable in the ways it needs to be.
      - oooyay 3y ago
        It's pinned to a Go version and these: https://github.com/dolthub/maphash/blob/main/hasher_test.go https://github.com/dolthub/maphash/blob/main/hasher_test.go
        tedunangst 3y ago
        That's annoying, but at least reliable. Thank you.

lukevp 3y ago

Could you provide more context on what’s disgusting? I’m not a go programmer but this looks like it’s just a wrapper around a library called ‘maphash’ and doesn’t do any of the hashing here? This is more of a service wrapper around that lib so that it can have a consistent seed value etc.?

brabel 3y ago
I suppose the use of unsafe pointers liberally is disgusting to some.
sidlls 3y ago
I wouldn't call it disgusting. It's just typical unsafe go code. The named return parameter in `getRuntimeHasher` is irritating, as all such parameters in go code are. If you're going to return values, do so explicitly in the return statement. In go code, seeing `return` at the end of a function does not mean that the function doesn't return any values, and that can lead to harder-to-read code.
- insanitybit 3y ago
  > It's just typical unsafe go code. But... just to hash something?
  - sidlls 3y ago
    Go has a lot of lower level internal implementations of things that unsafe code like this serves as a thin shim for. It’s an almost zero-cost abstraction to use such a builtin. I’m not saying it’s good: it just is what it is. It’s silly: but so is most of go. I’ve worked with this language for several years now: I don’t like it at all. It presents a facade of simplicity, but has all sorts of hidden issues that affect performance and behavior in surprising ways.
    - insanitybit 3y ago
      I think we're all saying the same thing then.
tedunangst 3y ago
Constructing a map (an opaque type) just so you cast it to a carefully duplicated struct so you can steal the function pointer is kinda nutso. Did you look at the linked code in maphash?

oooyay 3y ago

Disgusting is a word, but I'm guessing you'd say the same about most unsupported feature code (aka unsafe). You can make code like this reliable in the ways it needs to be while still being "unsafe". That's kind of table stakes when you start dabbling in unsupported things.

tedunangst 3y ago
And what does this code do to make itself reliable in the event the go authors change the internal structure of a map?
- rad_gruchalski 3y ago
  Nothing. That’s the trade off. What is not clear about “unsafe” in that context?
  - tedunangst 3y ago
    The part about making it reliable in the ways it needs to be.
    - oooyay 3y ago
      It's pinned to a Go version and these: https://github.com/dolthub/maphash/blob/main/hasher_test.go https://github.com/dolthub/maphash/blob/main/hasher_test.go
      - tedunangst 3y ago
        That's annoying, but at least reliable. Thank you.