716762 - startup cache needs a way of updating cached items

Status: NEW

(Core :: XPCOM, enhancement, P5)

Description

[John Daggett (:jtd)]

The startup cache code provides a simple mechanism for loading/saving items into the startup cache:

http://mxr.mozilla.org/mozilla-central/source/startupcache/StartupCache.h#59

Binary blobs with string keys are saved using PutBuffer and loaded via GetBuffer.  However, there's no way to invalidate and/or refresh a single item in the cache, the code currently asserts when this occurs.  Comment in the code:

  "PutBuffer will assert if the client attempts to insert a
   buffer with the same name as an existing entry."

There's an InvalidateCache mechanism but no other way of removing/replacing the contents of a given item.

Comment 1

[John Daggett (:jtd)]

Attachment: patch, allow startup cache items to be replaced

Add an optional boolean parameter to PutBuffer that indicates whether an override is allowed or not.  By default it's false, which is sort of the current semantics.  If true, no assertion will occur.

Looking over the code, I think this should work correctly (i.e. the updated item will be written out to the archive).

Comment 2

[Nathan Froyd [:froydnj]]

Can you write a test for this in TestStartupCache.cpp?  I think this only works because of the way nsZipArchive::BuildFileList inserts things into its hash table.  Would be nice if future cleanups in this area didn't break that functionality.

Do you have any estimate on how often duplicate keys will be inserted?  Frequently changing items could cause the startup cache to grow rather rapidly.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.

Comment 4

[Jens Stutte [:jstutte]]

(In reply to John Daggett (:jtd) from comment #1)

Add an optional boolean parameter to PutBuffer that indicates whether an
override is allowed or not. By default it's false, which is sort of the
current semantics. If true, no assertion will occur.

Looking over the code, I think this should work correctly (i.e. the updated
item will be written out to the archive).

The implementing code changed heavily, so I think this patch is obsolete. The comment is still there, so I assume the behavior wtr did not change, but I'd say it is a feature request, not a defect.