Details
-
Type:
Story
-
Status: Invalid
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: afw
-
Labels:
Description
Looking up keys by string,
src['somefield']
|
is about 60x slower than pre-computing the key and using that,
key=src.schema.asKey()
|
src.get(key)
|
We should be able to cache those keywords in a dict to normalize that access speed and so that users don't have to deal with pre-defining and carrying around a bunch of keys.
I'll attach a simple ipython file that demonstrates the speed difference. The output of one run of it is shown below:
$ ipython field_key_timing.py
|
Small schema, string lookup
|
1 loops, best of 3: 214 ms per loop
|
Small schema, asKey() lookup
|
100 loops, best of 3: 3.35 ms per loop
|
|
|
Large schema, string lookup
|
1 loops, best of 3: 201 ms per loop
|
Large schema, asKey() lookup
|
100 loops, best of 3: 3.18 ms per loop
|
Attachments
Issue Links
Activity
Something has changed since this ticket was filed (likely the swig-pybind11 change): the `[string]` lookup is now significantly faster:
$ ipython field_key_timing.py
|
Using matplotlib backend: MacOSX
|
Small schema, string lookup
|
100 loops, best of 3: 16.9 ms per loop
|
Small schema, asKey() lookup
|
100 loops, best of 3: 4.33 ms per loop
|
|
|
Large schema, string lookup
|
100 loops, best of 3: 16.3 ms per loop
|
Large schema, asKey() lookup
|
100 loops, best of 3: 4.4 ms per loop
|
New version of test file that includes .get(string) test, to compare bracket vs. get() performance for strings.
John Parejko, thanks for the reminder. I forgotten that I explicitly tried to fix this during the pybind11 conversion (DM-8716) but never got around to testing the performance improvement. It's not quite using a dict lookup - it's using a std::map lookup in C++ instead, so it's a guaranteed log(N) lookup instead of a usually constant-time lookup - but I think that's close enough. If you agree, I think we can close this as Invalid.
It's definitely an improvement, though it's still a factor of 4 different. I guess we can close it for now and see if it ends up causing a bottleneck anywhere?
Since this bubbled up in a filter that Yusra AlSayyad just made, I thought this was basically fixed with DM-11871 and maybe seemingly unrelated fixes. I just ran the attached test (very helpful!) on the latest weekly, and things seem reasonable:
$ ipython field_key_timing.py
|
Small schema, string lookup
|
16.7 ms ± 539 µs per loop (mean ± std. dev. of 7 runs, 100 loops each) |
Small schema, string-get lookup |
17.6 ms ± 1.42 ms per loop (mean ± std. dev. of 7 runs, 10 loops each) |
Small schema, asKey() lookup
|
9.61 ms ± 113 µs per loop (mean ± std. dev. of 7 runs, 100 loops each) |
Small schema, asKey() inline lookup
|
18 ms ± 275 µs per loop (mean ± std. dev. of 7 runs, 100 loops each) |
|
|
Large schema, string lookup
|
17.2 ms ± 758 µs per loop (mean ± std. dev. of 7 runs, 100 loops each) |
Small schema, string-get lookup |
16.7 ms ± 1.74 ms per loop (mean ± std. dev. of 7 runs, 100 loops each) |
Large schema, asKey() lookup
|
9.43 ms ± 207 µs per loop (mean ± std. dev. of 7 runs, 100 loops each) |
Large schema, asKey() inline lookup
|
18.6 ms ± 1.23 ms per loop (mean ± std. dev. of 7 runs, 100 loops each) |
I additionally added what I call "asKey() inline lookup" of the form: x.get(src.schema['parent'].asKey()).
So the string lookup isn't any slower than the key lookup at the moment, the performance gain is because you only look up the key once and reuse it in the loop, which is basically guaranteed to be faster. The fact that it's only a factor of x2 in performance is the surprising part!
I think we can close this ticket, but John Parejko should chime in if he's still seen performance problems related to this.
I found this in the pair coding tickets list and re-ran it:
Small schema, string lookup
|
12 ms ± 275 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
|
Small schema, string-get lookup
|
11.9 ms ± 310 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
|
Small schema, asKey() lookup
|
8.5 ms ± 378 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
|
|
|
Large schema, string lookup
|
12.2 ms ± 342 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
|
Small schema, string-get lookup
|
12 ms ± 463 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
|
Large schema, asKey() lookup
|
8.02 ms ± 44 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)
|
So some time in the last 2 years, string lookups got even faster and are only 50% slower than asKey(). I agree with Eli Rykoff that this is good enough; if John Parejko or anyone else disagrees, please re-open the ticket or file a new one.
Run with ipython field_key_timing.py.