Open
Bug 864944
Opened 11 years ago
Updated 2 years ago
Make PStorage actors from children with the same principal share a parent DOMStorage object
Categories
(Core :: DOM: Core & HTML, defect, P5)
Tracking
()
People
(Reporter: jdm, Unassigned)
References
Details
(Whiteboard: [MemShrink:P2] c=)
Attachments
(1 file)
Currently we create a brand new DOMStorageImpl for every StorageParent instance, causing us to re-cache everything on the first access. We should look into obtaining the relevant principal and using nsIDOMStorageManager to get a proper shared DOMStorage object in the parent instead.
Updated•11 years ago
|
Whiteboard: [MemShrink]
Reporter | ||
Updated•11 years ago
|
Version: unspecified → 18 Branch
Updated•11 years ago
|
blocking-b2g: --- → tef?
Reporter | ||
Comment 1•11 years ago
|
||
I've got code that I'm testing, but this appears harder than originally expected. We seem to be missing the code that ensures one storage object per origin.
Assignee: nobody → josh
Reporter | ||
Comment 2•11 years ago
|
||
Honza, the comment in nsIDOMStorageManager for getLocalStorageForPrincipal says "Returns instance of localStorage object for aURI's origin. This method ensures there is always only a single instance for a single origin.", which we assumed meant we could get a storage object that wouldn't require re-caching all of the keys and values on subsequent reloads. However, looking at the code, getLocalStorageForPrincipal has always created a brand new storage object since it was first introduced, and there does not appear to be any kind of origin -> storage object cache anywhere. Am I misunderstanding the purpose of the method?
Flags: needinfo?(honzab.moz)
Reporter | ||
Comment 3•11 years ago
|
||
As described.
Comment 4•11 years ago
|
||
(In reply to Josh Matthews [:jdm] from comment #2) > Honza, the comment in nsIDOMStorageManager for getLocalStorageForPrincipal > says "Returns instance of localStorage object for aURI's origin. This method > ensures there is always only a single instance for a single origin.", which > we assumed meant we could get a storage object that wouldn't require > re-caching all of the keys and values on subsequent reloads. However, > looking at the code, getLocalStorageForPrincipal has always created a brand > new storage object since it was first introduced, and there does not appear > to be any kind of origin -> storage object cache anywhere. Am I > misunderstanding the purpose of the method? Not sure I follow the question. In this old code nsDOMStorage caches all its entries inside it self. There is always new nsDOMStorage object for each windows.localStorage. We also preload the data to an inmemory sqlite table. This has changed in the new code - there is only one common cache for all objects.
Flags: needinfo?(honzab.moz)
Comment 5•11 years ago
|
||
(In reply to Honza Bambas (:mayhemer) from comment #4) > In this old code nsDOMStorage caches all > its entries inside it self. In it's DOMStorageImpl to be exact.
Comment 6•11 years ago
|
||
If you really want to go the way of saring DOMStorageImpl, you may want to keep a hash table of them by principal code base URI. The patch should be just used to prove the concept.
Reporter | ||
Comment 7•11 years ago
|
||
Given the amount of effort required to create a new per-origin cache and get the details of when to delete it correct, I don't think this should be the focus of our tef-blocking efforts. We should get as much bang for our buck from bug 864941 instead.
Updated•11 years ago
|
Whiteboard: [MemShrink] → [MemShrink] [tef-triage]
Comment 8•11 years ago
|
||
Leaving this nominated for now as we discuss bug 850175 and whether that will continue to block.
Whiteboard: [MemShrink] [tef-triage] → [MemShrink:P3] [tef-triage]
Whiteboard: [MemShrink:P3] [tef-triage] → [MemShrink:P2] [tef-triage]
Comment 9•11 years ago
|
||
Circling back to this, bug 850175 is taking it's course based on comment 7 this bug actually sounds like too much risk to take so late in v1.0.1 and our resources are better spent elsewhere. Tracking and flagging for scrum backlog.
blocking-b2g: tef? → -
tracking-b2g18:
--- → +
Whiteboard: [MemShrink:P2] [tef-triage] → [MemShrink:P2] c=
Reporter | ||
Updated•11 years ago
|
Assignee: josh → nobody
Comment 10•6 years ago
|
||
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046 Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5. If you have questions, please contact :mdaly.
Priority: -- → P5
Assignee | ||
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•