Tiny Set optimizations

[noxiouz]

1. bool is replaced with struct{} to save some memory in large sets
2. Use direct call of (R)Unlock in really plain simple methods like Exists, Len, Clear to speedup Set

with defer()

BenchmarkLen    50000000            52.8 ns/op
BenchmarkExists 20000000            93.4 ns/op
BenchmarkClear  10000000           215 ns/op

without defer()

BenchmarkLen    100000000           21.9 ns/op
BenchmarkExists 50000000            51.5 ns/op
BenchmarkClear  10000000           167 ns/op

[alexandercampbell-wk]

+1, looks good

[tylertreat-wf]

+1

[dustinhiatt-wf]

+1

[cep21]

The defer optimization is somewhat surprising for me. My only guess on why it's not so easy to optimize the same way is that defer() will also do the unlock if something inside panics, so if the code ever panics because of some bug users of the data structure would be unable to recover. Any other thoughts?

[noxiouz]

I can show you a lot of places in stdlib where defer is not used to unlock. On the other side I replaced defer only in functions without possibilities to panic and one point to return. It's hard to expect a situation where _, ok := m["a"] would panic. It's a really simple cases. Also there's some kind of research here: http://lk4d4.darth.io/posts/defer/

[alexandercampbell-wk]

Compile error on this line:

# github.com/Workiva/go-datastructures/set
../../workspace/src/github.com/Workiva/go-datastructures/set/dict_test.go:205: undefined: set
FAIL    github.com/Workiva/go-datastructures/set [build failed]

[alexandercampbell-wk]

Theoretically you could recover from a panic and continue using the datastructure, but whatever caused the panic probably left it in a bad state. I wouldn't trust the data at that point.

Defer is very convenient and aids safety in larger functions with multiple return sites, but I agree that in simple functions like these, the performance improvement is worth explicitly unlocking before the return.

[alexandercampbell-wk]

Thanks for fixing that issue! +1

[dustinhiatt-wf]

+1