Live data from Hacker News

SeaHash: A fast, portable hash function in Rust

docs.rs

81–90 of 116 posts

Re: SeaHash: A fast, portable hash function in Rust

#81
post #70

Earlier quoted context omitted.

Nobody is putting effort into propaganda about unsafe because the community is already very strongly averse to this, and is careful about writing unsafe code. It's not a problem. If it becomes a problem (I doubt it) we can put effort into it. People learn about the language through discussion or documentation, and both of these venues actively discourage unsafe. The one resource out there that teaches unsafe code in…

Have you ever been bitten by unsafe code? IMHO any metrics would be gamed and prioritized above actually quality. As actual quality isn't suffering, why add the unsafe metric in at all? Seems like paranoia not born out of experience.

> Have you ever been bitten by unsafe code?

Yeah. Most often in FFI code (when the invariants are much harder to uphold). Rarely when writing unsafe abstractions. The few times I remember this happening with abstractions is due to really old code that broke in a compiler upgrade (pre-1.0).

> IMHO any metrics would be gamed and prioritized above actually quality.

Yeah, there have been discussions in the past about a "safe code" badge for crates and stuff like this, and the conclusion is that it might discourage people from using unsafe code where they actually should be.

Re: SeaHash: A fast, portable hash function in Rust

#82

Earlier quoted context omitted.

Yeah, this exists, and would be interesting. I again think that it's a bit too extreme a solution to be baked into Rust itself, but I'd love a SPARKish Rust variant.

I'd like both. SPARK's stuff was ported to Ada 2012. It can be done for Rust as well. The trick is to make it optional so people don't have to pay attention to it. Maybe even have editors filter it out for people not paying attention to it. At the least, it being used in standard library and OS API's would let it enforce correct usage of those in debug/testing mode. 80/20 rule says that should have some impact given…

Yeah, me too. However, I think we should wait for the formal verification of Rust to be completed before trying this. While it is possible to make something SPARKish without complete formal verification, it's probably better to build it using concepts learned during the formal verification.

Re: SeaHash: A fast, portable hash function in Rust

#83
post #79

Earlier quoted context omitted.

> We're probably going to see "unsafe" code that assumes good behavior on the part of the caller. I have yet to see any of this. I have noticed that it's harder to write correct unsafe code when it comes to parallelism and FFI, but parallelism has always been a hard problem and the FFI problems generally come from the fact that you need to know the invariants being upheld on the other end, which is trickier. But for…

> I have yet to see any of this. mem::forget-pocalypse was this. (Rc/Arc, Vec::drain, thread::scoped) Any UB bug that results from an overflow is kind've implicitly this. BTreeMap::range still has an UB bug from trusting the caller! I literally asked you to fix it! https://github.com/rust-lang/rust/issues/33197 Bugs happen man.

> mem::forget-pocalypse was this.

I would say that this is from a time when the invariants were not understood. In particular, the fact that leaking is safe to do in safe code was not known.

(The invariants are still not completely understood, but there's work to specify that, and IMO they're understood enough to be able to avoid unsafe bugs)

> BTreeMap::range still has an UB bug from trusting the caller!

Fair :) I'd completely forgotten about that one.

Re: SeaHash: A fast, portable hash function in Rust

#84
post #79

Earlier quoted context omitted.

> I have yet to see any of this. mem::forget-pocalypse was this. (Rc/Arc, Vec::drain, thread::scoped) Any UB bug that results from an overflow is kind've implicitly this. BTreeMap::range still has an UB bug from trusting the caller! I literally asked you to fix it! https://github.com/rust-lang/rust/issues/33197 Bugs happen man.

> mem::forget-pocalypse was this. I would say that this is from a time when the invariants were not understood. In particular, the fact that leaking is safe to do in safe code was not known. (The invariants are still not completely understood, but there's work to specify that, and IMO they're understood enough to be able to avoid unsafe bugs) > BTreeMap::range still has an UB bug from trusting the caller! Fair :) I'd…

> I would say that this is from a time when the invariants were not understood.

Yeah, but it's not like "oh this is an obvious thing to consider trusting the caller about". It's an exceptionally niche problem that you'd only know about if someone told you about it. Especially since a Rust programmer shouldn't be expected to write unsafe code often, if ever!

Similarly: not trusting traits to be implemented correctly. Not trusting closures to not-unwind.

Re: SeaHash: A fast, portable hash function in Rust

#85
post #61

Earlier quoted context omitted.

While I mostly agree with you, I'd like to play devil's advocate: The Rust core team is relatively relaxed about the community's usage of `unsafe`. I say this because they do not seem to be interested in actively discouraging it's usage. i.e. `unsafe` is discouraged in documentation, not via tools. "Hey please don't use `unsafe` unless you know what you're doing". Is like writing a comment in Javascript, `function(x…

Tooling could encourage this by requiring that if a module or crate contains unsafe code, its name has to contain "unsafe".

That sounds horrifying (to me).

I'm not a Rust user. I've never written a line of Rust code in my life. So my opinion is not worth much. And I'm exaggerating a bit. But still.

I don't think I can explain my real reason for objecting to such a thing; it's more emotional than rational, but at least let me give one rational-sounding example of where I think even you would agree such a policy would backfire.

Some library comes out, becomes fairly popular. Eventually a version 2 is released, but the new version has a few lines of unsafe code, whereas the old one did not. Maybe it's to enable a new feature, or maybe it's for performance. Maybe the author was properly paranoid and did a full-fledged correctness proof that his code had no bugs compared to the documentation, and then verified the proof with several different interactive proof assistants. (This is getting a bit unrealistic, but bear with me for a few more sentences.)

But he now has to rename his crate, instantly making the upgrade process for existing users way more annoying. And to add insult to injury, the new name is wrong! There's nothing unsafe about his code, as far as users are concerned. The only trouble is that the Rust compiler could not prove it safe. As you pointed out yourself, this is not surprising: it can't even cope with a linked list! Sophomores in college implement these things, but the borrow checker can't cope with them. So of course unsafe is needed.

And if version 3 finds a workaround, so that unsafe is no longer needed, do you rename it yet again? People think Java's checked exceptions are annoying, but surely this is far worse?

Re: SeaHash: A fast, portable hash function in Rust

#86
post #74
post #55

Earlier quoted context omitted.

> The same code the parent poster highlighted [1] is > undefined behavior in C/C++ (with standard types). So > really no language has the ability to express those > concepts. Your doing pointer casts and possibly unaligned > dereferences at the same time. This has zero consistency > between CPU vendors. It's undefined behavior in Rust, too. Rust code that type-puns an unaligned pointer into an integer would crash on…

Type punning is a mess in C because you have to jump through hoops to make it legal. The language subsequently failed to provide ergonomic solutions to the very real problems type punning solves in a systems language. Type punning is perfectly allowed in Rust, I'm not aware of any lints against it. Although you need to use an annotation to specify the struct layout algorithm to do it "correctly" for custom types. We…

Type-punning is the not the same thing as deriving an object pointer through casting. Or relying on the rule concerning the equivalence of structures containing the same initial sequence of sub-members.

Specifically, the following common macro in C is _not_ type-punning, at least not the kind I had in mind.

  #define container_of(ptr, type, member) \
     (type *)((char *)(ptr) - offsetof(type, member))
Neither is the idiomatic BSD library. It's all valid, well-defined code, as long as they're not being abused to hide undefined shenanigans.

In C, as long as the last access to an object had the same type as the type you're accessing that object from (and provided it's the same object), that's perfectly well-defined. People think that this is an aliasing violation in C, but it's not. Aliasing issues only come into play when there are side-effects (including order of evaluation) that you implicitly depend on but that the compiler cannot see. That issue is too complex to bother discussing in fine detail here, but suffice it to say that Rust either has similar undefinedness issues, or it assumes any pointer however derived can alias even inside an unsafe block and therefore cannot perform the same optimizations that a C compiler can. I doubt the latter is the case given that rustc relies on the LLVM backend.

Note that the general rule in C is that all pointers of the same type can alias, so if you derive two object pointers to the same type through explicit conversion (casting) or implicit conversion, and as long as they're actually referring to the same object, then as long as the last access is through the same type as the last store all is well-defined.[1] This is not type-punning. If this wasn't allowed by the language there wouldn't even be any use for casting at all. The cast is a way to stop the optimizer in its tracks and ensure that it doesn't fubar otherwise correct code.

The aliasing issue typically comes into play when you access at least one object through a structure or union, and there's no union definition in scope that hints that the layout is such that the sub-members of the union or structure might alias. Though the dereferenced expressions might have the same type, the dereference isn't occurring through pointers with the proper compatible type. This is one of the few cases where the compiler isn't required to assume that accesses might alias. And this is why the C standard requires those types of evaluations to occur through pointers to a union.

Type-punning, at least the kind I had in mind, is violating the core rule that access can only happen through an object with the same type as the last store. This is type-punning:

  unsigned long l = 1;
  unsigned *i;
  i = (unsigned *)&l;
  printf("i:%d\n", *i);
The access through i has a different type than the store to l.

An example that isn't type-punning per se, but raises the aliasing issue,

  struct foo {
    int i;
  }

  int add(struct foo *fp, int *ip) {
    int i;
    fp->i = 0;
    i = *ip;
    return fp->i + i;
  }

  int main(void) {
    struct foo f;
    return add(&f, &f.i);
  }
In add(), the C compiler isn't required to assume that &fp->i might alias ip and so might reorder the statements. If the hidden dependency on ordering didn't exist, the code could otherwise be okay (that's why I don't call it type-punning).

(Note: I'm having trouble using the asterisk without bolding everything. Please keep that in mind.)

The above can be made correct simply by casting:

  int add(struct foo *fp, int *ip) {
    int i;
    *(int *)&fp->i = 0;
    i = *ip;
    return *(int *)&fp->i + i;
  }
because now the initial store occurs through type pointer-to-integer, same as the type of ip. IOW, the compiler must assume that the store and loads might alias and cannot reorder things.

As you can see, this is a much more contrived scenario. It's not as common as you'd think. It's most common when type-punning--storing through one type and accessing through another. Don't type-pun and you won't run into this issue very often, if ever. It's not even that common when using the typical C OOP-like inheritance tricks. It can happen in a silent and deadly way, but that requires some serious hackery. Don't pretend that C is Java and you're unlikely to write such code. One of the common places this occurs in practice is type-punning struct sockaddr, struct sockaddr_storage, etc. That's a very unique situation for many reasons. But as optimizations in compilers improve it is admittedly an increasing problem; it's a loaded gun, for sure.

FWIW, C11 defines type-punning in a footnote 95 of section 6.5.2.3p3.

  If the member used to read the contents of a union object is
  not the same as the member last used to store a value in the
  object, the appropriate part of the object representation of
  the value is reinterpreted as an object representation in
  the new type as described in 6.2.6 (a process sometimes
  called ‘‘type punning’’). This might be a trap
  representation.
This definition is even narrower than mine, but it still comports with the core rule about loads occurring through the same type as stores.

[1] Storing a value through char is also okay as long as you ensure the representation is valid. Which is trivial when dealing with the standard fixed-width unsigned types. And access is always valid through char. That's why sometimes you'll see a seemingly superfluous cast through (char *). It's not necessarily type-punning; it might be used because a pointer-to-char can alias _anything_, and that can be useful as a barrier to prevent an optimizer from deciding two expressions might not alias.

Re: SeaHash: A fast, portable hash function in Rust

#87
post #71

Earlier quoted context omitted.

Why would hard-coding the ability to access a slice of bytes as ints into the compiler be safer than a well-encapsulated unsafe code abstraction? We used to implement things like vectors directly in the compiler, but it was a big headache for no gain. Writing actual code is way easier than writing code to generate LLVM IR. Anyway, there is a commonly-used crate for this: byteorder. Had I written the library, I would…

I actually talked to Lattner about the isize thing a few months back -- according to him it's fine to overflow while doing GEP because llvm shouldn't care if you pass in negative offsets to represent really big positive ones.

My understanding is that overflow is pretty much fine and dandy in LLVM unless your frontend opts into the nsw flag.

Re: SeaHash: A fast, portable hash function in Rust

#88
post #84

Earlier quoted context omitted.

> mem::forget-pocalypse was this. I would say that this is from a time when the invariants were not understood. In particular, the fact that leaking is safe to do in safe code was not known. (The invariants are still not completely understood, but there's work to specify that, and IMO they're understood enough to be able to avoid unsafe bugs) > BTreeMap::range still has an UB bug from trusting the caller! Fair :) I'd…

> I would say that this is from a time when the invariants were not understood. Yeah, but it's not like "oh this is an obvious thing to consider trusting the caller about". It's an exceptionally niche problem that you'd only know about if someone told you about it. Especially since a Rust programmer shouldn't be expected to write unsafe code often, if ever! Similarly: not trusting traits to be implemented correctly.…

Fair. I'm not saying that your average Rust programmer will be able to deal with unsafe code immediately. But I do think that at this stage the list of things you can and cannot rely on (and the invariants you must uphold) is clear enough that in theory you could make a checklist to deal with this. The nomicon provides much of the background for folks wanting to figure this out and write unsafe code.

These days I've been writing a lot of unsafe code (for FFI) and I do want to get around to penning a concise guide (or just expanding the nomicon). But I'm mostly waiting for the unsafe code subteam to figure out a couple things before doing this (specifically, the exact boundaries of rust's noalias UB becomes important in FFI and this is not specified yet).

But yeah, it's not necessarily obvious. I'd like to make it easier to get this understanding of unsafe code though.

Re: SeaHash: A fast, portable hash function in Rust

#89
post #78

Earlier quoted context omitted.

> Also, looking at the byteorder crate, I wouldn't be surprised if it's even slower than the simpler and correct loop I posted elsethread. read_num_bytes in that create uses copy_nonoverlapping, which I assume is analogous to memcpy in C. That's a very round-a-bout and inefficient way to accomplish the task, and likely patterned after similarly bad C code. It wasn't patterned after any C code. ptr::copy_nonoverlappin…

Namely, concrete sizes are given, so the compiler backend can optimize this down to simple loads and stores, which is going to do better than the bit-shifting approach It can't optimize it down to simple loads and stores unless it can prove that it's aligned. If it can't optimize it to a simple load, it has to check for alignment. If it has to check for alignment, it's unlikely to be faster than the byte-loading func…

> It can't optimize it down to simple loads and stores unless it can prove that it's aligned. If it can't optimize it to a simple load, it has to check for alignment. If it has to check for alignment, it's unlikely to be faster than the byte-loading function.

I had edited my comment after-the-fact to include the "on x86" qualification.

> And that's what I meant by saying effort and code complexity is better spent refactoring the algorithm at a higher level than trying to micro-optimize such a small operation.

Your advice is overspecified. If you want to make something faster, then build a benchmark that measures the time you care about and iterate on it. If "micro optimizations" make it faster, then there's nothing wrong with that. I once doubled the throughput of a regex implementation by eliminating a single pointer indirection in the inner loop. It doesn't get any more micro then that, but consumers are no doubt happier with the increased throughput. In general, I find most of your hand waving about performance curious. You seem keen on making a strong assertion about performance, but the standard currency for this sort of thing is benchmarks.

I did all of this with byteorder when I built it years ago. I'll do it again for you.

    $ curl -sOL https://gist.github.com/anonymous/042d05e1e480b89434a673b30534efd8/raw/d2c9a4516a57c26da23c8beaffd5ad583da0a889/Cargo.toml
    $ curl -sOL https://gist.github.com/anonymous/042d05e1e480b89434a673b30534efd8/raw/d2c9a4516a57c26da23c8beaffd5ad583da0a889/lib.rs
    $ RUSTFLAGS="--emit asm" cargo bench
    test bit_shifting ... bench:   1,999,496 ns/iter (+/- 53,427)
    test type_punning ... bench:     476,105 ns/iter (+/- 11,920)
(The `RUSTFLAGS="--emit asm"` dumps the generated asm to target/release/deps.)

The benchmark reads 1,000,000 64 bit integers from a buffer in memory and sums them.

Analyzing the hotspots of each benchmark using `perf` is instructive. For type_punning:

    $ perf record target/release/deps/benchbytes-a1cc37a72d289957 --bench type_punning
    $ perf report
The corresponding asm is:

    cmpq	$7, %rsi
    jbe	.LBB4_10
    movq	(%rbx), %rcx
    addq	(%rcx,%rax), %rdi
    addq	$8, %rax
    addq	$-8, %rsi
    cmpq	%rax, %rdx
    ja	.LBB4_6
Notice how tight this loop is. In particular, we're dealing with a single simple load to read our u64. Now let's repeat the process for bit shifting:

    $ perf record target/release/deps/benchbytes-a1cc37a72d289957 --bench bit_shifting
    $ perf report
The hotspot's corresponding asm is:

    .LBB5_6:
    	cmpq	$7, %rsi
    	jbe	.LBB5_10
    	movzbl	(%rdx,%rbx), %ecx
    	movzbl	1(%rdx,%rbx), %eax
    	shlq	$8, %rax
    	orq	%rcx, %rax
    	movzbl	2(%rdx,%rbx), %ecx
    	shlq	$16, %rcx
    	orq	%rax, %rcx
    	movzbl	3(%rdx,%rbx), %eax
    	shlq	$24, %rax
    	orq	%rcx, %rax
    	movzbl	4(%rdx,%rbx), %ecx
    	shlq	$32, %rcx
    	orq	%rax, %rcx
    	movzbl	5(%rdx,%rbx), %eax
    	shlq	$40, %rax
    	orq	%rcx, %rax
    	movzbl	6(%rdx,%rbx), %ecx
    	shlq	$48, %rcx
    	movzbl	7(%rdx,%rbx), %edi
    	shlq	$54, %rdi
    	orq	%rcx, %rdi
    	orq	%rax, %rdi
    	addq	%rdi, %r12
    	addq	$8, %rbx
    	addq	$-8, %rsi
    	cmpq	%rbx, %r11
    	ja	.LBB5_6
It's no surprise that the type punning approach is faster here. (N.B. Compiling with `RUSTFLAGS="-C target-cpu=native"` seems to permit some auto-vectorization to happen, but I don't observe any noticeable improvement to the benchmark times for bit_shifting. In fact, it seems to get a touch slower.)

I could be reasonably accused of micro-optimizing here, but I do feel like reading 1,000,000 integers from a buffer is a pretty generalizable use case, and the performance difference here in particular is especially dramatic. Finding a real world problem that this helps is left as an exercise to the reader. (I've exceeded my time budget for a single HN comment.)

> It's beyond dispute that the gains from SeaHash primarily come from how it refactored its inner loop to operate on a 64-bit word instead of 8 8-bit words.

Do you feel anyone has contested this point? I note your use of the word "primarily." If type punning gives a 10% boost to something that is already fast, do you care? If not, do you think other people might care? If they do, then what exactly is your point again?

Note that I am responding to your criticism of byteorder in particular. I don't really know whether the OP's optimization of reading little-endian integers is actually worth while or not. I would hazard a guess, but would suspend certainty until I saw a benchmark. (And even then, it is so incredibly easy to misunderstand a benchmark.)

Re: SeaHash: A fast, portable hash function in Rust

#90
post #69

Earlier quoted context omitted.

Have you looked at Siphash? I remember reading recently that murmur has been found to have some predictable hashes independent of salts but a lot of big names still use it since it's fast and has good properties.

When SipHash was presented at CCC, it was alongside the proof of concept attacks against MurmurHash and CityHash. See the "Attacks" section of [0]. Murmur was notable at the time for its use in Java and Ruby. Ruby has since moved to SipHash-2-4, while Java (OpenJDK) has thrown up its hands in disgust at the problem and created a binary tree fallback mode for its HashMaps[1]. Which at this point I'm pretty confident i…

It's a bit disingenuous to say java has given up and created a binary tree fallback mode. It doesn't actually switch the whole hash table into a binary tree, but rather it switches the linked list inside each bucket into a binary tree, and only when a certain threshold is passed.
Post reply on HN