Add test which attempts to encounter a concurrent resize and retrieval

[jhinch]

I noticed that one of the areas of the code which had low test coverage was the BinEntry::Moved branch in node.rs. This test is designed to exercise that codepath. At least on my machine it seems to exercise the codepath but it is non-deterministic whether it will actually encounter the path.

[jhinch]

Of course it decides to not hit the codepath in CI :P. How would you feel about me adding a unit test for the find method so it can be tested deterministically?

[jonhoo]

A unit test for that would be fantastic!

[jonhoo]

I also wonder whether you might be able to trigger more resizes by forcing all the keys into a single bucket like we do here:

flurry/tests/basic.rs

Lines 88 to 104 in 5fe084b

	use std::hash::{BuildHasher, Hasher};
	struct OneBucketState;
	struct OneBucketHasher;
	impl BuildHasher for OneBucketState {
	type Hasher = OneBucketHasher;
	fn build_hasher(&self) -> Self::Hasher {
	OneBucketHasher
	}
	}
	impl Hasher for OneBucketHasher {
	fn write(&mut self, _bytes: &[u8]) {}
	fn finish(&self) -> u64 {
	0
	}
	}

[jhinch]

At least when testing locally, using the one bucket hasher makes the BinEntry::Moved be encountered less often. I believe this is because the transfer function will transfer entire runs of entries when it can, so you actually want quite shallow bins to trigger the desired behaviour

[jonhoo]

This looks great! I do still wish we had a reliable way to check that the next_table pointer in Moved was set correctly, which basically requires a resize to take place while we run a find. Maybe one way to do that is to set up a very large table in the test, then call reserve with a value large enough to trigger a resize in a separate thread, and then immediately do gets on the main thread?

[jhinch]

I would guess the only way to do it reliably is using a tool like loom to test each of the permutations

[jonhoo]

I think the strategy I outlined above should actually work pretty reliably, but yes, ultimately we'll want something like loom as well (#34).

[jonhoo]

@jhinch would you be interested in also trying to implement the test strategy above?

[jhinch]

Apologies. I thought your comments were more speculative of how things could be done rather than of something which should be changed as part of this PR. I have switched it over to use .reserve() instead and increased it to resize to 32K to increase likelihood of a resize being triggered

[jonhoo]

No worries. It was a bit of a speculative comment :p

I think you'll first want to do many many inserts, and then start the thread that does resizing. Otherwise the resizes will all finish very quickly, and you may not even see it.

[jhinch]

It looks like I've introduced a memory leak in the unit tests I added. I'm working through the problem now.

[jonhoo]

Ah, yes, the tests you've written never actually free any of the memory they allocate since you do not create a Table at any point (which is the thing that actually calls defer_destroy on stuff in its bins).

[jhinch]

Alright, all tests are passing again and merge conflicts have been resolved

[jonhoo]

Great, thanks!