be smarter about syscalls when reading pref files

RESOLVED FIXED in Firefox 45, Firefox OS v2.5

Status

()

RESOLVED FIXED
3 years ago
a year ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

(Depends on: 1 bug)

unspecified
mozilla45
Points:
---

Firefox Tracking Flags

(firefox45 fixed, b2g-v2.5 fixed)

Details

Attachments

(2 attachments)

Comment hidden (empty)
(Assignee)

Comment 1

3 years ago
Created attachment 8680092 [details] [diff] [review]
part 1 - ask the prefs file for its size directly

Calling nsIInputStream::Available on nsIFileInputStreams is relatively
expensive, as it requires three system calls: one to determine the
current file position, one to seek to the end file position, and one to
rewind to the previous file position.

We can do better by asking the file for its size directly, prior to
opening the stream.  This only requires one system call, stat, and is
thus superior--at least in considering the number of system calls.

(I remember that full stats on Windows are relatively slow--at least compared
to Linux full stats--but I'd think that getting the file size for an open fd
requires touching roughly the same stuff inside the kernel.  No, I haven't
measured this...)
(Assignee)

Comment 2

3 years ago
Created attachment 8680093 [details] [diff] [review]
part 2 - keep track of how much pref file we have read

Looking at a preference file read with strace typically looks like:

open("...", O_RDONLY) = X
...
read(X, "...", SIZE) = SIZE
read(X, "...", SIZE) = 0
...

There's no reason to call Read() and make another syscall to determine
there's no data left for reading.  We can keep track of how much we've
read at minimal cost and thus determine for ourselves when we are done.

(This is a more obvious win, as we're eliminating extra work, guaranteed.)
Attachment #8680093 - Flags: review?(n.nethercote)
(Assignee)

Comment 3

3 years ago
Comment on attachment 8680092 [details] [diff] [review]
part 1 - ask the prefs file for its size directly

See comment 1 for some details.
Attachment #8680092 - Flags: review?(n.nethercote)
Comment on attachment 8680092 [details] [diff] [review]
part 1 - ask the prefs file for its size directly

Review of attachment 8680092 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me
Attachment #8680092 - Flags: review?(n.nethercote) → review+
Comment on attachment 8680093 [details] [diff] [review]
part 2 - keep track of how much pref file we have read

Review of attachment 8680093 [details] [diff] [review]:
-----------------------------------------------------------------

rs=me
Attachment #8680093 - Flags: review?(n.nethercote) → review+

Comment 7

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0f532182498f
https://hg.mozilla.org/mozilla-central/rev/66296b0eec30
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45

Updated

a year ago
Depends on: 1385755
You need to log in before you can comment on or make changes to this bug.