search

catapult-project / catapult

Deprecated Catapult GitHub. Please instead use http://crbug.com "Speed>Benchmarks" component for bugs and https://chromium.googlesource.com/catapult for downloading and editing source code..
https://chromium.googlesource.com/catapult
BSD 3-Clause "New" or "Revised" License
1.92k stars 564 forks source link

Histograms - ImportDicts LookupDiagnostic? #4451

Closed simonhatch closed 6 years ago

simonhatch commented 6 years ago

Was doing some light profiling on the dev server, noticed that seemingly more than 50% of the time for importDicts seems to be in the diagnostic lookup. If someone can confirm and see if there's anything super easy to fix.

Specifically ResolveSharedDiagnostics, replacing self[name] = diag with dict.setitem(self, name, diag) seems to make the time disappear.

@benshayden @eakuefner

benshayden commented 6 years ago

Replacing self[name] = diag with dict.__setitem__ bypasses the consistency checks in DiagnosticMap.__setitem__, which arguably might be safe considering ResolveSharedDiagnostics is only called in ImportDicts, but I'm confused because that wouldn't affect LookupDiagnostic. LookupDiagnostic is just a getitem, which should be extremely fast.

simonhatch commented 6 years ago

I'll rename this, at the time I didn't know what specifically was slow but edited the description later with the line that I tried swapping out.