Closed Bug 839468 Opened 8 years ago Closed 7 years ago

[OS.File] Read/write JSON objects

Categories

(Toolkit :: OS.File, defect)

defect
Not set
normal

Tracking

()

RESOLVED INCOMPLETE

People

(Reporter: Yoric, Unassigned)

References

()

Details

(Keywords: perf)

Attachments

(1 file)

At the moment, the main usage scenario for OS.File is for reading or writing JSON files. Given that implementing jankless JSON conversion is much trickier than expected, I believe that we should provide methods that can directly read/write JSON:
- OS.File.read should accept an option |type:"json"|;
- OS.File.writeAtomic should accept an option |type:"json"|;
- both functions should ensure that json (de)serialization is async and as jankless as possible.
Why does this functionality need to go in OS.File rather than being implemented in a standalone module?
It could definitely be a standalone module. However, right now, I feel that a module OS.JSONFile with two methods |read| and |writeAtomic| would be a little weird. In addition, if we ever decide to add more data structures than ArrayBuffer and JSON (say XML document), such a design would lead us to add yet another module OS.XMLFile, etc.

What do you think?
Taras, it seems to me that you suggested we should implement this. Is that still valid?
Flags: needinfo?(taras.mozilla)
(In reply to David Rajchenbach Teller [:Yoric] from comment #3)
> Taras, it seems to me that you suggested we should implement this. Is that
> still valid?

I'm with nathan, this should be some sort of a utility on top of os.file, not core functionality
Flags: needinfo?(taras.mozilla)
Well, we could have a module such as
 OS.Data
or
 OS.DataFile

At the moment, however, I have a few difficulties envisioning how this can be architectured outside of the core of OS.File in a manner such that we can take advantage of bug 852187 et al. I guess we could have a dedicated OS.DataFile worker, but that sounds a little overkill, doesn't it?
JSON is not particularly suited to be used as a streaming format. I can see in the case of postMessage() the only thing it really needs to do is transmit in chunks from the worker and wait for all the chunks to arrive on the main thread. (At this point I don't know why worker traffic requires disk I/O to start with... does anyone have a link for me to clarify things?)

But how will you know that you received the last chunk after X atomic writes? Will you keep trying JSON.parse() until it stops yielding errors? Of course, there are plenty of SAX-style JSON parsers out there that reconstruct JSON, usually newline (/\r?\n/g) delimited, but these usually hurt performance.

If we'd use a 'safe', fast, binary and streaming-capable intermediate format to stream JSON (amongst others) between two points directly, would that work for workers?
Assignee: nobody → dteller
Flags: needinfo?(dteller)
Status: NEW → ASSIGNED
(In reply to Mike de Boer [:mikedeboer] from comment #7)
> JSON is not particularly suited to be used as a streaming format. I can see
> in the case of postMessage() the only thing it really needs to do is
> transmit in chunks from the worker and wait for all the chunks to arrive on
> the main thread. (At this point I don't know why worker traffic requires
> disk I/O to start with... does anyone have a link for me to clarify things?)

As discussed over IRC, it doesn't. We're using postMessage() to send JSON to a worker because we want that worker to handle I/O.

> But how will you know that you received the last chunk after X atomic
> writes? Will you keep trying JSON.parse() until it stops yielding errors? Of
> course, there are plenty of SAX-style JSON parsers out there that
> reconstruct JSON, usually newline (/\r?\n/g) delimited, but these usually
> hurt performance.

I actually don't understand the question.
Assignee: dteller → nobody
Flags: needinfo?(dteller)
Status: ASSIGNED → NEW
We're not sure we want this feature, so I'll mark the bug as INCOMPLETE. We'll reopen it when we're sure of what we want.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.