extensions: swap EventListeners container, improving speed and safety [chromium/src : main]
0 views
Skip to first unread message
Sean Maher (Gerrit)
Nov 25, 2024, 12:41:02 PM11/25/24
to Kelvin Jiang, Tricium, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Kelvin Jiang
New activity on the change![]( "Open in Gerrit")
Related details
Attention is currently required from:
- Kelvin Jiang
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kelvin Jiang (Gerrit)
Nov 26, 2024, 3:09:41 PM11/26/24
to Sean Maher, Tricium, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Kelvin Jiang added 1 comment![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kelvin Jiang (Gerrit)
Nov 27, 2024, 7:33:46 PM11/27/24
to Sean Maher, Tricium, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Sean Maher
Kelvin Jiang added 4 comments![]( "Open in Gerrit")
Patchset-level comments
Kelvin Jiang . resolved
thanks Sean! A few high level questions before I take a closer look (also since a lot of this CL looks boilerplate-y)
Commit Message
Line 15, Patchset 16 (Latest):
Currently, as outlined in the Bug, according to SSM, we are seeing
~0.4% of main thread CPU being consumed by a O(n^2) operation in
DispatchEventToListeners(). This is especially problematic because it
is on the critical path. This CL changes that function to be O(n log n).Kelvin Jiang . unresolved
curious: can you share the actual time savings here before/after via profiling? (just something quick in the code that prints time should be fine)
File extensions/browser/api/web_request/extension_web_request_event_router.h
Line 475, Patchset 16 (Latest):
using Set = std::map<EventListener::ID, std::unique_ptr<EventListener>>;Kelvin Jiang . unresolved
Essentially we're using this map except everything is wrapped so we can have the Listener back-reference it, to check a listener is only in one map at a time (wrapper is basically to guarantee these via calls?)
is it possible to do something like `class Listeners : public std::map<EventListener::ID, std::unique_ptr<EventListener>>` ?
Line 358, Patchset 16 (Latest):
class ID {Kelvin Jiang . unresolved
can we keep this as a struct since we're not adding any new data and the only new method added is public anyway?
Related details
Attention is currently required from:
- Sean Maher
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sean Maher (Gerrit)
Jan 8, 2026, 1:20:44 PM (21 hours ago) Jan 8
to Kelvin Jiang, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Sean Maher abandoned this change![]( "Open in Gerrit")
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages