Closed Bug 504217 Opened 12 years ago Closed 5 years ago

nsZipReaderCache is pointless

Categories

(Core :: Networking: JAR, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
blocking-basecamp -

People

(Reporter: taras.mozilla, Assigned: mwu)

References

Details

(Whiteboard: [necko-would-take])

Attachments

(2 files)

Every time we open a file in a jar file, we stat the filesystem, this sucks up about 100ms on wince for no good reason.

It doesn't sound like there is a good reason to support modifying the jar files while mozilla is running. Infact I doubt this works too well as old nsZipArchive would linger in existing InputStreams.

I propose nuking the cache and making it into something that merely manages existing nsZipArchive objects.
As long as there's a way to "manually" clear the manager, this sounds fine...
Summary: nsJARCache is pointless → nsZipReaderCache is pointless
I settled my differences with nszipreadercache in bug 504864
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Got bitten by bug 637286 for not taking care of this sorry excuse for a cache sooner.
Assignee: nobody → tglek
Status: RESOLVED → REOPENED
Depends on: 637286
Resolution: FIXED → ---
Attachment #525097 - Flags: review?(mwu)
Attached patch bonus nukingSplinter Review
I think we should also remove this cache hitrate crap while at it.
Attachment #525098 - Flags: review?(mwu)
If we had telemetry, we could probably figure out if this causes excessive jar reopening for most users.
(In reply to comment #6)
> If we had telemetry, we could probably figure out if this causes excessive jar
> reopening for most users.

we have the histogram support now. What exactly would you measure?
I think we want to nuke zipreadercache and add a simple, small ziparchivecache which will eliminate our hack to detect omnijars at open time.
Assignee: tglek → mwu
Just thinking out loud here. There is actually a real use case for the zipreadercache: keepingJARs open across several uses of the JAR protocol handler (which is where most JAR accesses go). Without the zipreadercache, we would be opening JARs, and reading their index for each and every jar:some.jar!/path url with a different "path". At least, that's how the JAR protocol handler works at the moment.
(In reply to Mike Hommey [:glandium] from comment #9)
> Just thinking out loud here. There is actually a real use case for the
> zipreadercache: keepingJARs open across several uses of the JAR protocol
> handler (which is where most JAR accesses go). Without the zipreadercache,
> we would be opening JARs, and reading their index for each and every
> jar:some.jar!/path url with a different "path". At least, that's how the JAR
> protocol handler works at the moment.

Yes, but at the moment it does more than this.
nsZipReaderCache is hurting B2G.  See bug 806383.
blocking-basecamp: --- → ?
I don't think this should block basecamp. Fixing this directly involves some risk on all platforms and the discussion in bug 806383 suggest that this probably isn't actually hurting that much.
blocking-basecamp: ? → -
If and when we remove jar caching we might want to at least still cache the file descriptors for jar files, else we'll need to do IPC to open them on parent and schlep them back to child for every JARChannel (as I'm implementing in bug 815523: jar caching means this happens once per app run instead of per resource).

See also bug 817202--we're currently doing main thread I/O for fopen() in asyncOpen and zipcache amortizes that too.
Status: REOPENED → RESOLVED
Closed: 12 years ago5 years ago
Resolution: --- → FIXED
Whiteboard: [necko-would-take]
You need to log in before you can comment on or make changes to this bug.