Open Bug 1231711 Opened 4 years ago Updated 3 months ago

Port OS.File to C++

Categories

(Toolkit :: OS.File, defect)

defect
Not set

Tracking

()

People

(Reporter: Yoric, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [qf:p3:responsiveness])

Putting this on the radar. Would solve bug 1208199 which has baffled us (in its previous incarnations) for 1-2 years, plus save memory.
Better for projects using rust to depend on the 'oxidation' build support bug, instead of blocking it.
No longer blocks: oxidation
Depends on: oxidation
How much of the complexity of OS.file is the implementation of all the IO (managing file descriptors etc), and how much of it is exposing a nice API to WebIDL (managing promises, etc)? If we do this in Rust, we're still going to need to implement the API layer in C++.
Frankly, most of the complexity is exposing the nice API to WebIDL. That's the main thing that has refrained me from working on this so far. Well, that and the chronic lack of time.
(In reply to David Teller [:Yoric] (please use "needinfo") from comment #3)
> Frankly, most of the complexity is exposing the nice API to WebIDL. That's
> the main thing that has refrained me from working on this so far. Well, that
> and the chronic lack of time.

In that case, this bug is tantamount to "Port OS.File to C++". Is that something we actually want to do?
It might be something we want to do, I'm not sure. Also, this bug can also be "once Rust and WebIDL can speak natively together in Gecko, port OS.File to Rust".
(In reply to David Teller [:Yoric] (please use "needinfo") from comment #5)
> It might be something we want to do, I'm not sure. Also, this bug can also
> be "once Rust and WebIDL can speak natively together in Gecko, port OS.File
> to Rust".

Except that we have no intention of making Rust and WebIDL speak natively together in the foreseeable future, which is why I think this bug is as-filed is kinda misleading.
Summary: Port OS.File to Rust → Port OS.File to C++
(Unless there is some large surface of IO machinery that we would write in Rust that doesn't already exist in Gecko C++).
No longer depends on: oxidation

Adding to the qf triage queue as this may have perf improvement.

Whiteboard: [qf]

I'd have thought that the slow bit would be the code that hits the disk, not the JS interpreter.

So I would argue that we should collect some evidence that OS.File is slow because it's in JS before we commit to rewriting it in native code. Is it possible to prove that by collecting profiles?

Whiteboard: [qf] → [qf:p3:responsiveness]
See Also: → 981789, 975702, 1408729

(In reply to Chris Pearce [:cpearce (GMT+13)] from comment #9)

I'd have thought that the slow bit would be the code that hits the disk, not the JS interpreter.

So I would argue that we should collect some evidence that OS.File is slow because it's in JS before we commit to rewriting it in native code. Is it possible to prove that by collecting profiles?

When we tested, OS.File was slow (at least during startup) because loading the JS code from a worker required hitting the disk while everything else in Firefox was busy doing the same thing, to load modules, UI resources, etc. This caused serious cache contention which, when we measured with Telemetry, could last more than 5 seconds for ~5 percentiles of users.

We got out of this by fast-pathing a few critical bits of OS.File into C++, which means that some of the code of OS.File is implemented twice, once in fast-path, once in slow-path. Not the nicest/most reliable of things, especially in presence of concurrency.

No longer blocks: 1549386
You need to log in before you can comment on or make changes to this bug.