Closed
Bug 528651
Opened 15 years ago
Closed 14 years ago
Component registrations not correctly cached leading to re-registering every component on every startup
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
INVALID
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: mossop, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [ts])
Attachments
(1 file)
1.50 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
I'm not sure why this isn't causing a Ts regression but since bug 511761 we aren't caching files in the component registry leading us to try re-register them on every startup unless you are running debug builds.
Reporter | ||
Updated•15 years ago
|
Assignee: nobody → dtownsend
Whiteboard: [ts]
Reporter | ||
Comment 1•15 years ago
|
||
We don't get a modification time for the module's file anymore except in debug builds so these tests are failing all the time. Seems like we only care about the modification time on debug builds too so there isn't much point in getting them and it is unclear when they might be 0 so this just caches the module's file regardless. Pushing this to try to look at timings but this should be correct I think. At least I can now see files listed in compreg.dat.
Attachment #412346 -
Flags: review?(benjamin)
Reporter | ||
Updated•15 years ago
|
Status: NEW → ASSIGNED
(In reply to comment #0) > I'm not sure why this isn't causing a Ts regression This is a really, really, really good question. We have the registration and all that stuff there because it's "expensive" to do at startup -- so why are we being saved from that?
Reporter | ||
Comment 3•15 years ago
|
||
(In reply to comment #2) > (In reply to comment #0) > > I'm not sure why this isn't causing a Ts regression > > This is a really, really, really good question. We have the registration and > all that stuff there because it's "expensive" to do at startup -- so why are we > being saved from that? One potential argument from this is that almost all of the components we are talking about here are JavaScript ones. Even though we would be loading them everytime their JS should be in the fastload so that should save us somewhat. I'm also looking and I'd actually guess that maybe half of them are used during startup anyway so for those we probably lose very little by re-registering them everytime. That said I still think we should have seen some regression based on this, and more loss than the kind of win I'd have expected from the rest of bug 511761, but according to bug 511761 comment 33 we actually saw a win overall. I'm also not seeing very much interesting on the try talos runs from this, though the numbers are pretty messy to really draw any conclusions there.
Comment 4•15 years ago
|
||
Comment on attachment 412346 [details] [diff] [review] patch rev 1 Wow, yes.
Attachment #412346 -
Flags: review?(benjamin) → review+
Reporter | ||
Comment 5•15 years ago
|
||
Pushed: http://hg.mozilla.org/mozilla-central/rev/5795fd10ba3d Will keep an eye on the talos results
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 6•15 years ago
|
||
I can't see any obvious signs of Ts improvement from this patch. In fact it is possible that it regressed Ts on windows but there are far too few data points to tell for sure at this point. Just for fun I'm going to try pushing a patch that completely removes component caching to the try server to see what it does.
Comment 7•15 years ago
|
||
(In reply to comment #6) > I can't see any obvious signs of Ts improvement from this patch. In fact it is > possible that it regressed Ts on windows but there are far too few data points > to tell for sure at this point. It regressed MIN and MED dirty profile Ts on Vista, for about 2 and 3 percents, respectively.
Reporter | ||
Comment 8•15 years ago
|
||
As I would have hoped a patch that disables compreg.dat completely shows a healthy Ts regression, but this patch is certainly causing a regression too so I've backed it out for now. Deeper debugging is suggesting that there is still an issue here though. http://hg.mozilla.org/mozilla-central/rev/0f5b586b91b6
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reporter | ||
Updated•14 years ago
|
Assignee: dtownsend → nobody
Updated•14 years ago
|
blocking2.0: --- → ?
Comment 10•14 years ago
|
||
We don't cache these at all anymore, due to changes in bug 568691.
Status: REOPENED → RESOLVED
Closed: 15 years ago → 14 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•