Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ArcIntern: add from_str() specialization for ArcIntern<String> #60

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

eliad-wiz
Copy link

This is useful in order to allow looking for interned str without having to create owned value first.

simple benchmarks show nice improvement in case the str was already interned:

    cached/String/short     time:   [13.598 ms 13.783 ms 13.994 ms]
                            change: [-0.5323% +1.5518% +3.7562%] (p = 0.15 > 0.05)
                            No change in performance detected.
    Found 14 outliers among 100 measurements (14.00%)
      7 (7.00%) high mild
      7 (7.00%) high severe
    cached/String/short-from_ref
                            time:   [13.582 ms 13.757 ms 13.955 ms]
                            change: [-1.5349% +0.4010% +2.5499%] (p = 0.70 > 0.05)
                            No change in performance detected.
    Found 12 outliers among 100 measurements (12.00%)
      3 (3.00%) high mild
      9 (9.00%) high severe
    cached/String/short-from_str
                            time:   [9.3047 ms 9.4529 ms 9.6175 ms]
                            change: [-1.9867% +0.4488% +2.9423%] (p = 0.72 > 0.05)
                            No change in performance detected.
    Found 15 outliers among 100 measurements (15.00%)

if the string is not interned we do end up with additional lookup, but other functions (new/from_ref) can be used if that's an issue.

This is useful in order to allow looking for interned str
without having to create owned value first.

The internal BorrowStr trait was used since we can't just
have blanket implementation for T: Borrow<Q> due to conflicting
implementations (since T: Borrow<T>)

Signed-off-by: Eliad Peller <eliad.peller@wiz.io>
(the alternative is using HashSet)

Signed-off-by: Eliad Peller <eliad.peller@wiz.io>
the string building is not part of the test, and taking them
out will allow benchmarking against other scenarios more easily

Signed-off-by: Eliad Peller <eliad.peller@wiz.io>
using the from_str benchmark we can see that it indeed gives
performance boost if the strings were already interned, as it
avoids a redundant string allocation.

cached/String/short     time:   [13.598 ms 13.783 ms 13.994 ms]
                        change: [-0.5323% +1.5518% +3.7562%] (p = 0.15 > 0.05)
                        No change in performance detected.
Found 14 outliers among 100 measurements (14.00%)
  7 (7.00%) high mild
  7 (7.00%) high severe
cached/String/short-from_ref
                        time:   [13.582 ms 13.757 ms 13.955 ms]
                        change: [-1.5349% +0.4010% +2.5499%] (p = 0.70 > 0.05)
                        No change in performance detected.
Found 12 outliers among 100 measurements (12.00%)
  3 (3.00%) high mild
  9 (9.00%) high severe
cached/String/short-from_str
                        time:   [9.3047 ms 9.4529 ms 9.6175 ms]
                        change: [-1.9867% +0.4488% +2.9423%] (p = 0.72 > 0.05)
                        No change in performance detected.
Found 15 outliers among 100 measurements (15.00%)

Signed-off-by: Eliad Peller <eliad.peller@wiz.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant