perf(audeze): answer every info capability from one status read - #550
perf(audeze): answer every info capability from one status read#550AdrianKuriata wants to merge 1 commit into
Conversation
getDeviceStatus() returns battery, chatmix, sidetone and eq together, but getBattery() and getChatmix() each called it. On a Maxwell 2 that made -b -m -o json take 2.83 s instead of 1.41 s for one set of values. Cached for 500 ms, keyed on the handle since one instance serves every attached device, and mutex-guarded since it is shared.
6c494f7 to
f77991f
Compare
|
This one is clearly worth it, thanks. Setting read_at_ after the read instead of before is the right call. Please drop the mutex. HeadsetControl is single threaded, there is no other lock in the whole codebase and I would rather not start here. One thing I want handled though: the cache only expires by time, never when something is written. Right now only getBattery and getChatmix go through it and neither is affected by the setters, so nothing is broken today. But MaxwellStatus also carries sidetone, equalizer and noise filter, and setSidetone, setEqualizerPreset and setNoiseFilter all change exactly those. The moment someone adds a getter for one of them on top of statusFor, a set followed by a get within 500ms silently returns the old value and it will look like a firmware quirk, not like a cache. Either clear last_status_ in those setters or put a clear warning on statusFor. Small thing, the doc comment sits above STATUS_REUSE_WINDOW so it reads as if it documents the constant. Move it down to statusFor. I have no Maxwell 2 here, so I only verified that it builds and the tests pass and took your measurements as given. For the intermittent status reads please open a separate issue, otherwise it gets lost in here. |
getDeviceStatus() sends 15 init packets plus 6 status requests, each after a 60 ms
delay, and returns battery, chatmix, sidetone, equalizer and noise filter
together. But getBattery() and getChatmix() each call it, so asking for both pays
twice for one set of numbers.
Audeze Maxwell 2 (0x3329:0x4b28), release build:
With the status reused for 500 ms the second one drops to 1.41 s. The window is
much shorter than the 1.4 s read it saves, so nothing gets a value staler than its
own read would have produced. Reading both from one pass also makes them a
coherent snapshot instead of two readings 1.4 s apart.
Keyed on the device handle since DeviceRegistry keeps one instance per device
class, and mutex-guarded since that instance is shared - it is the only mutable
state in the class. Happy to drop the lock if you consider concurrent calls out of
scope.
headsetcontrol_tests passes.
Unrelated but visible if you test this: status reads on this device fail
intermittently on master too - five consecutive -b -m -o json runs gave
battery = -1, 74, -1, -1, 74 - so a flaky reading is not a regression from this
patch.
Independent of #549, different file and different cause; either can go in without
the other.