Skip to content

Commit

Permalink
auto merge of #15399 : kballard/rust/rewrite_local_data, r=alexcrichton
Browse files Browse the repository at this point in the history
This was motivated by a desire to remove allocation in the common
pattern of

    let old = key.replace(None)
    do_something();
    key.replace(old);

This also switched the map representation from a Vec to a TreeMap. A Vec
may be reasonable if there's only a couple TLD keys, but a TreeMap
provides better behavior as the number of keys increases.

Like the Vec, this TreeMap implementation does not shrink the container
when a value is removed. Unlike Vec, this TreeMap implementation cannot
reuse an empty node for a different key. Therefore any key that has been
inserted into the TLD at least once will continue to take up space in
the Map until the task ends. The expectation is that the majority of
keys that are inserted into TLD will be expected to have a value for
most of the rest of the task's lifetime. If this assumption is wrong,
there are two reasonable ways to fix this that could be implemented in
the future:

1. Provide an API call to either remove a specific key from the TLD and
   destruct its node (e.g. `remove()`), or instead to explicitly clean
   up all currently-empty nodes in the map (e.g. `compact()`). This is
   simple, but requires the user to explicitly call it.
2. Keep track of the number of empty nodes in the map and when the map
   is mutated (via `replace()`), if the number of empty nodes passes
   some threshold, compact it automatically. Alternatively, whenever a
   new key is inserted that hasn't been used before, compact the map at
   that point.

---

Benchmarks:

I ran 3 benchmarks. tld_replace_none just replaces the tld key with None
repeatedly. tld_replace_some replaces it with Some repeatedly. And
tld_replace_none_some simulates the common behavior of replacing with
None, then replacing with the previous value again (which was a Some).

Old implementation:

    test tld_replace_none      ... bench:        20 ns/iter (+/- 0)
    test tld_replace_none_some ... bench:        77 ns/iter (+/- 4)
    test tld_replace_some      ... bench:        57 ns/iter (+/- 2)

New implementation:

    test tld_replace_none      ... bench:        11 ns/iter (+/- 0)
    test tld_replace_none_some ... bench:        23 ns/iter (+/- 0)
    test tld_replace_some      ... bench:        12 ns/iter (+/- 0)
  • Loading branch information
bors committed Jul 31, 2014
2 parents 2eb3ab1 + e65bcff commit 75a39e0
Show file tree
Hide file tree
Showing 2 changed files with 386 additions and 125 deletions.

0 comments on commit 75a39e0

Please sign in to comment.