Closed Bug 920804 Opened 6 years ago Closed 6 years ago
Frame Message Manager
Currently the message manager keeps a the message listeners in a simple array, and as such needs to iterate over the full array to find potential listener for a message. This patch replaces this array by a hash table indexed on the message name, so we get O(1) access to the list of matching listeners. With this patch I see the overhead in nsFrameMessageManager::ReceiveMessage() now very constantly under 1ms, with some outliers cases that I'm still investigating.
Attachment #810201 - Flags: feedback?(bugs)
We have the patch for this somewhere. reviewed patch even, I think.
Or maybe not. At least I can't find it. But jlebar certainly wrote something like this.
Comment on attachment 810201 [details] [diff] [review] wip patch Yeah, I think we need to have something like this. MessageManager is being used in a bit different way than I originally thought :) Remove the /**/ and fix the TODO etc. And -w patch will help reviewing.
Attachment #810201 - Flags: feedback?(bugs) → feedback+
Cleaned up and complete patch sent to try: https://tbpl.mozilla.org/?tree=Try&rev=77543ea0498d
Fixing unused variables: https://tbpl.mozilla.org/?tree=Try&rev=e48b47dddb04
With less ws, but as much test failures: https://tbpl.mozilla.org/?tree=Try&rev=d8b772a60e60
Attachment #810201 - Attachment is obsolete: true
I added some logging to understand why M-1 is failing. It seems that Specialpowers sync calls to getBoolPref() is failing because no listener is found for the SPPrefService message even though it's added correctly. I'm not sure yet how this patch breaks the world.
Olli, If you could help with the try failure that would be great!
Status: NEW → ASSIGNED
Whiteboard: [c= p= s= u=]
Attachment #814453 - Flags: feedback?(bugs) → review?(bugs)
I'm having hard time to see other problems than leaking listeners in unlink (need to actually delete the arrays, and that leads to deleting the nsMessageListenerInfo objects).
I assume that will still fail. I don't understand...
Fabrice, so the main difference to your patch is to fix ReceiveMessage to not return early in case there are no listeners, but to call the parent message manager, and to make memory reporter stuff working with the new setup. (I also fixed the coding style of the recently landed memory reporter code)
Attachment #821399 - Flags: review?(fabrice) → review+
Unfortunately had to back this out as it conflicted against https://hg.mozilla.org/integration/mozilla-inbound/rev/e4c43a39c0f6 when mozilla-central was merged to b2g-inbound. Backout: https://hg.mozilla.org/integration/b2g-inbound/rev/fb0173b73836
Fabrice, could you update the patch. I may not be online too much today. Changes in https://hg.mozilla.org/integration/mozilla-inbound/diff/e4c43a39c0f6/content/base/src/nsFrameMessageManager.cpp are trivial.
Note, -w of v2 and v3 is exactly the same.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
koi+ blocking a blocker
blocking-b2g: koi? → koi+
This doesn't apply cleanly to b2g26. Please post a branch-specific patch or request uplift for whatever else this depends on.
Ryan, can you check if https://bugzilla.mozilla.org/attachment.cgi?id=8341582 from bug 933711 applies cleanly?
It does not.
You need to log in before you can comment on or make changes to this bug.