Closed
Bug 506392
Opened 15 years ago
Closed 12 years ago
Dealing with manifest files is slow
Categories
(Toolkit :: Startup and Profile System, defect, P1)
Toolkit
Startup and Profile System
Tracking
()
RESOLVED
INCOMPLETE
People
(Reporter: taras.mozilla, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [ts])
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
i'm seeing 77ms on cold startup on a 7200rpm hd to iterate over all manifest files and process them.
on wince it's 400ms
Comment 1•15 years ago
|
||
Do you have a breakdown of how much time is directory listing and how much is reading?
Reporter | ||
Comment 2•15 years ago
|
||
http://people.mozilla.org/~tglek/manifests.txt
is on the desktop. On Wince dir reading takes up more time due to lack of vfs cache, also messing about with urls is more expensive on wince, so there is cpu-related slowdown there.
Reporter | ||
Updated•15 years ago
|
Whiteboard: [ts]
Comment 3•15 years ago
|
||
Taras says combining manifests inside a single jar would help.
Comment 4•15 years ago
|
||
(In reply to comment #3)
> Taras says combining manifests inside a single jar would help.
er, combining all manifests into a single one, covering all jars.
that'd have to be done as a packaging step.
Updated•15 years ago
|
Priority: -- → P1
Comment 5•15 years ago
|
||
This isn't just a Linux problem, right?
Reporter | ||
Comment 6•15 years ago
|
||
(In reply to comment #5)
> This isn't just a Linux problem, right?
no
Updated•15 years ago
|
OS: Linux → All
Hardware: x86 → All
Comment 7•15 years ago
|
||
Adding yet another option to JarMaker.py to write out to a single manifest file should be rather easy.
http://mxr.mozilla.org/mozilla-central/source/config/JarMaker.py#173 is the code in question, don't have cycles to do that myself, though.
Reporter | ||
Comment 8•15 years ago
|
||
(In reply to comment #7)
> Adding yet another option to JarMaker.py to write out to a single manifest file
> should be rather easy.
>
> http://mxr.mozilla.org/mozilla-central/source/config/JarMaker.py#173 is the
> code in question, don't have cycles to do that myself, though.
That does look reasonable. I was originally thinking of doing this at build time to deal with not merging the locale manifest. Do you have a suggestion on how/where to best deal with locale manifest. I'm happy to do the grunt work here.
Comment 9•15 years ago
|
||
We could do some makefile foo in the locales Makefiles to disable the single manifest there.
Comment 10•15 years ago
|
||
How's this for a start? It doesn't handle locale manifests specially.
On my Macbook reading the manifests is 25s of cold startup. With this patch, the single manifest is 15s.
Questions:
* The single manifest's basename should be what?
* Re: comment 9, where do those locale Makefiles live?
Comment 11•15 years ago
|
||
How would Fennec's plan to ship multiple locales in a single package affect locale manifest handling? I guess it depends on how we build/package the locales.
Comment 12•15 years ago
|
||
Re the patch, the addition of --single-manifest should be conditional, perhaps some
ifndef DISTINCT_MANIFEST
MAKE_JARS_FLAGS += --single-manifest
endif
It should stay off for external customers, IMHO, so maybe it should be in autoconf.mk.in. More of a question for ted.
For multi-locale builds, we need to think a bit, as we need that for regular l10n repacks.
We had to define DISTINCT_MANIFEST in most of http://mxr.mozilla.org/mozilla-central/find?text=&kind=text&string=locales%2FMakefile.in, IIRC, even for the branding dirs there, too. And the equivalent on comm-central and mobile-browser, and possibly branches of central as fits.
Comment 13•15 years ago
|
||
I think if this makes things faster, why wouldn't everyone want it? Let's not try to please everyone unless someone specifically comes up with an issue.
Comment 14•15 years ago
|
||
The name should be chrome.manifest. We cannot do this unless we have a solution for either not merging the locale manifest, or repacking the locale manifest.
Reporter | ||
Comment 15•15 years ago
|
||
I believe the most effective way to deal with manifest slowness would be to solve bug 506841 via some fastload-type cache that would cache all of the manifests and not touch files in chrome dir unless some compreg scenario occurs.
Comment 16•15 years ago
|
||
(In reply to comment #10)
> On my Macbook reading the manifests is 25s of cold startup. With this patch,
> the single manifest is 15s.
ms, not s, d'oh.
(In reply to comment #15)
> I believe the most effective way to deal with manifest slowness would be to
> solve bug 506841
Then what's the status of this bug? Simply dependent on bug 506841 with no other work? Or wontfix per the suboptimal proposed solution in comment 4?
Reporter | ||
Comment 17•15 years ago
|
||
(In reply to comment #16)
> (In reply to comment #10)
> > On my Macbook reading the manifests is 25s of cold startup. With this patch,
> > the single manifest is 15s.
>
> ms, not s, d'oh.
>
> (In reply to comment #15)
> > I believe the most effective way to deal with manifest slowness would be to
> > solve bug 506841
>
> Then what's the status of this bug? Simply dependent on bug 506841 with no
> other work? Or wontfix per the suboptimal proposed solution in comment 4?
wontfixed this, should work on bug 511761 instead. My vision is to have the compreg/whatever(bug 511761) act as a guard for chrome cache(and everything else). Furthermore, a reworked/unified mmaped fastload cache would be a perfect place to dump the cached manifest data. Unfortunatelly reworking fasl will take quite a while, in the meantime i think we should cache manifests into a discrete file in profile dir.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → WONTFIX
Reporter | ||
Comment 18•15 years ago
|
||
We should cache manifests in startup cache
Comment 19•13 years ago
|
||
I noticed this same slowdown when profiling locally by the way on Windows. I get about 100ms of my startup time from parsing manifests. :(
Comment 20•13 years ago
|
||
bbondy, things have changed significantly since this was last discussed: we no longer enumerate any directories looking for manifests (which was one of the big slowdowns). Can you provide more details about what you're seeing?
Comment 21•13 years ago
|
||
bsmedberg, I was originally seeing this about twice as high as it actually was since ParseManifest is called recursively, but you can see from this stat that it's still pretty significant.
nsComponentManagerImpl::RereadChromeManifests
Calls: 1
Max: 0.036:36
Avg: 0.036:36
Min: 0.036:36
Total: 0.036:36
All of this time is taken up inside ManifestParser.cpp in the function ParseManifest.
Comment 22•13 years ago
|
||
those are milliseconds, so 36ms. Not huge but worth fixing.
Comment 23•13 years ago
|
||
Is it possible to break down how much of that is iowait and how much is us doing the dumb-but-necessary XPCOM registration for each manifest directive?
Comment 24•13 years ago
|
||
Ya sorry I'm still looking deeper, that was just an intermediate result.
Comment 25•13 years ago
|
||
I haven't looked at ways to optimize this yet, but the biggest thing that sticks out to me is:
~21ms is spent in nsComponentManagerImpl::ManifestXPT
- FileLocation's constructor is ~3ms
- FileLocation.GetData is ~4ms
- data.Copy(buf, len); is ~2ms
- xptiInterfaceInfoManager::GetSingleton()->RegisterBuffer(buf, len); ~12ms
Comment 26•13 years ago
|
||
Sounds like most of the time is just parsing the XPT file and producing the in-memory data. bsmedberg and I talked about making the xpidl compiler produce C structs that we could link directly into libxul instead of parsing XPT files, but it never actually went anywhere. Seems like a lot of work for 12ms, but I don't know what percentage of startup that is.
Comment 27•13 years ago
|
||
> but I don't know what percentage of startup that is.
Only about 1% but it still helps the bottom line. I'll profile further to find out exactly where the problem is before implementing anything but that's a cool idea.
Comment 28•13 years ago
|
||
Be careful not to blow all the improvement on additional data relocations.
Updated•12 years ago
|
Status: REOPENED → RESOLVED
Closed: 15 years ago → 12 years ago
Resolution: --- → INCOMPLETE
You need to log in
before you can comment on or make changes to this bug.
Description
•