Shard the STRINGS in the intern cache for better parallel scalability

[mmp]

(This is an improvement to the approach in #162 that is, however, probably not yet ready to merge. Posting it for discussion.)

The 20% of runtime in egg::util::intern with #162 is still pretty high. This PR instead shards the cache to reduce mutex contention in the presence of multithreading. With it, in the context of my application, it further reduces the percentage of time in intern() by a factor of 4 with a corresponding benefit to overall performance:

As it stands currently, this PR unfortunately increases the size of Symbol to 8 bytes, which is probably not desirable. One option might be to hijack the high bits of the value in Symbol to store the shard index, if that would be acceptable.

[mmp]

(Another issue, perhaps, is the use of the same hash function both to choose a shard and then for inserting strings to the hash table. That could presumably potentially lead to an unexpectedly high number of hash collisions in a worst case scenario.)

[mwillsey]

I agree, I'd rather not increase the size of Symbol. If this is a continued perf bottleneck, we could consider switching to a purpose-built interning solution (many exist in Rust, but I'm not familiar with all of them).

[mwillsey]

Just leaving an idea here for now: You could "inline" < 3 byte strings into symbol pretty easily, which would require no synchronization for most variables (since most are very short).

Redefine Symbol as a i32 newtype. If the sign bit is set, then just read the bottom three bits as the string. I guess the string would be 0 terminated, or you could stash the length in the top byte as well.

EDIT: ah nevermind, this makes it impossible to implement Symbol::as_str(). You'd have to convert to a full String, which would be a more invasive change.

[mwillsey]

Ok nvm I figured it out. Could you try this branch instead on your workload? If your symbols are < 3 bytes, then they should be super fast! https://github.com/egraphs-good/egg/tree/tiny-symbols

[mmp]

Nice! I just got around to trying this with my workload; to my surprise it doesn't seem to give a performance benefit, FWIW.

[mwillsey]

Thanks! Interesting that there is no speedup. I’ll try to merge something like this approach then. Are your variable names all long or something?

[mmp]

Ah, yes-for the example I was testing with, it happened that they were all >4 characters. Trying another one that has a mix of short ones and a few longer ones, I see a 10% reduction in overall wallclock time, so this is a definite win for that case, since it does a bunch of other stuff besides call into egg.

[mmp]

And now with actual profiling.
Before:

After:

So that's about 10x and pushes intern() down into the noise. (The above doesn't exactly line up with the 10% overall reduction reported above, but close enough.)

[mwillsey]

Great! The latest commit on tiny-symbols now implements both of these techniques, and it keeps the Symbol size to 4 bytes :). Barring any crazy results from you, I'll merge that and close this PR.

[mwillsey]

Hi Matt! I've taken this idea and combined it with something inspired by this post and made the symbol_table crate.

Commit ed1faf1 (and the upcoming 0.8 release) will use this for string interning, which should solve your perf issues once and for all!