Evaluate Storage Options for Session Data

RESOLVED FIXED in Firefox 2 beta1

Status

()

enhancement
RESOLVED FIXED
14 years ago
13 years ago

People

(Reporter: dietrich, Assigned: dietrich)

Tracking

({fixed1.8.1})

2.0 Branch
Firefox 2 beta1
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox2 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: 181b1+)

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

14 years ago
Need to determine pros/cons of using the current text format vs. XML, SQLite or RDF.
Assignee

Updated

14 years ago
Blocks: 328154
Assignee: nobody → dietrich
Flags: blocking-firefox2+
Status: NEW → ASSIGNED
Assignee

Comment 1

13 years ago
The current implementation uses an ini file to store the session data.

This format is not typical of our current data storage approaches, but is not unprecedented (eg: profiles.ini). It provides these benefits:

- standard well-known format
- easily machine- and human-readable

Some downsides to using ini:

- not ideal for representing hierarchal data
- Yet Another File Format

SQLite: It may be advantageous to move the data into SQLite for consistency and integration reasons, but AFAICT there is no overwhelming requirement to do so at this time.

RDF: The project seems to be trending away from using RDF, in favor of mozStorage. The RDF APIs are cumbersome to work with, and have performance issues when working with large datasets. (Using mozStorage-backed-RDF, might solve the perf question?)

XML: http://www.tbray.org/ongoing/When/200x/2006/01/08/No-New-XML-Languages

(SWAG-ing at 0 days until conversation dictates otherwise)
Whiteboard: 0d

Comment 2

13 years ago
> XML: http://www.tbray.org/ongoing/When/200x/2006/01/08/No-New-XML-Languages
That page discusses entirely different thing (not that I think that XML is the right choice here).

I would like the toSource/eval[InSandbox] variant considered seriously (because if there are any shortcomings with it, I'd like them to be pointed out to me). It's only slightly less readable than an INI format (why do we care about its readability anyway?) and it is results in significally easier code.
Assignee

Comment 3

13 years ago
>> XML: http://www.tbray.org/ongoing/When/200x/2006/01/08/No-New-XML-Languages
> That page discusses entirely different thing (not that I think that XML is the
> right choice here).

It discusses reasons why not to create a new XML vocabulary. How is that not applicable to this discussion?

> I would like the toSource/eval[InSandbox] variant considered seriously (because
> if there are any shortcomings with it, I'd like them to be pointed out to me).
> It's only slightly less readable than an INI format (why do we care about its
> readability anyway?) and it is results in significally easier code.

Yes, the code would be simpler and more compact. I'll swag it for A2. Are there instances of other parts of the project doing storage this way?

WRT to readability: What I was trying to get at is that while ini doesn't provide any major benefits, there had been no overwhelming case for changing it.
Whiteboard: 0d → 1d

Comment 4

13 years ago
(In reply to comment #3)
> >> XML: http://www.tbray.org/ongoing/When/200x/2006/01/08/No-New-XML-Languages
> It discusses reasons why not to create a new XML vocabulary. How is that not
> applicable to this discussion?
That article talks about other kinds of languages. You can see this from the point author tries to make (there should be multiple interoperable implementations, years to develop the language, complex validating tools, etc.) Anyways I don't think XML is appropriate here either, so I don't know why I made that comment.
--
AFAIK no other code in tree uses toSource/evalInSandbox today, which is why I'm not sure if this approach has downsides I'm unaware of.
Assignee

Comment 5

13 years ago
Testing using toSource/eval for storing the session data. This allows us to remove the INI parsing/serializing code, modestly decreasing the size and complexity of the patch for 328159. Needs to be updated to use evalInSandbox.
re-targetted at beta 1
Target Milestone: Firefox 2 alpha2 → Firefox 2 beta1
Assignee

Updated

13 years ago
Whiteboard: 1d → [swag: .5d]
Assignee

Comment 7

13 years ago
Attachment #218920 - Attachment is obsolete: true
Attachment #226766 - Flags: review?(mconnor)

Comment 8

13 years ago
Should you really intend change the storage format, make sure to not leave any references to the INI format (i.e. change or remove the file extension). And then you could/should also get rid of the capitalized fields (i.e. use windows instead of Window, tabs instead of Tab, etc.).

Out of curiosity: has this change been discussed anywhere?
Assignee

Comment 9

13 years ago
(In reply to comment #8)
> Should you really intend change the storage format, make sure to not leave any
> references to the INI format (i.e. change or remove the file extension). And
> then you could/should also get rid of the capitalized fields (i.e. use windows
> instead of Window, tabs instead of Tab, etc.).

Thanks, will do.

> Out of curiosity: has this change been discussed anywhere?

Here on the bug, and on the wiki. Some benefits:

- no extra de/serialization at load, save and API
- makes it easy for extension authors to convert to other formats (and removes a step: deserializing from Ini)
- removes more code from both classes (this is only a plus if the "JS component size hurts Ts" theory is true)
Assignee

Updated

13 years ago
Attachment #226766 - Flags: review?(mconnor)
Assignee

Comment 10

13 years ago
Attachment #226766 - Attachment is obsolete: true
Attachment #226837 - Flags: review?(mconnor)
Attachment #226837 - Flags: review?(mconnor) → review+
Assignee

Comment 11

13 years ago
Checked in on trunk.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED

Updated

13 years ago
Whiteboard: [swag: .5d] → [swag: .5d] 181b1+
Assignee

Comment 12

13 years ago
Comment on attachment 226837 [details] [diff] [review]
fixes simon's comments

Working fine on trunk (after fixing bug 342889, which is already approved for branch).
Attachment #226837 - Flags: approval1.8.1?
Comment on attachment 226837 [details] [diff] [review]
fixes simon's comments

You are cleared to land on runway 181, taxi left and contact apron control on 171.9. G'day.
Attachment #226837 - Flags: approval1.8.1? → approval1.8.1+
Assignee

Updated

13 years ago
Keywords: fixed1.8.1
Whiteboard: [swag: .5d] 181b1+ → 181b1+
You need to log in before you can comment on or make changes to this bug.