801376 - [OS.File] Make loading symbols lazy

Status: RESOLVED FIXED

(Toolkit Graveyard :: OS.File, defect)

Description

[David Teller [:Yoric] - still alive but not very active]

At the moment, all the symbols are loaded eagerly from libc. However, there are good chances that 
1/ most symbols will not be used during the execution of OS.File on a worker thread;
2/ even the "official" file I/O thread that backs off-main-thread IO will certainly not use most of these symbols until rather late in the execution of the process.

We should therefore make sure that symbols are loaded lazily.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

This should help bring memory use of OS.File down.

Comment 2

[David Teller [:Yoric] - still alive but not very active]

Attachment: Loading symbols lazily

Try: https://tbpl.mozilla.org/?tree=Try&rev=3c528d16ddf7

Comment 3

[David Teller [:Yoric] - still alive but not very active]

Attachment: Loading symbols lazily, v2

Changes from v1:
- added missing documentation;
- made sure that all foreign functions are now defined lazily.

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 818747 [details] [diff] [review]
Loading symbols lazily, v2

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

Have you checked to see whether this affects memory use/startup time?  I can see OS.File initialization taking up time on Android pageload profiles...

::: toolkit/components/osfile/modules/osfile_unix_back.jsm
@@ +37,5 @@
>           declareFFI = aDeclareFFI.bind(null, libc);
>         } else {
>           declareFFI = SysAll.declareFFI;
>         }
> +       let declareLazyFFI = SharedAll.declareLazyFFI;

You could do |let declareLazy = ...| for consistency.  Optional.

Comment 5

[David Teller [:Yoric] - still alive but not very active]

(In reply to Nathan Froyd (:froydnj) from comment #4)
> Comment on attachment 818747 [details] [diff] [review]
> Loading symbols lazily, v2
> 
> Review of attachment 818747 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Have you checked to see whether this affects memory use/startup time?  I can
> see OS.File initialization taking up time on Android pageload profiles...

No, I haven't checked yet.
I believe that it should improve both memory use and startup time, because we probably use only few of the primitives during startup.

Comment 6

[David Teller [:Yoric] - still alive but not very active]

Attachment: Loading symbols lazily, v3

Fixed a symbol that was defined twice, causing errors under Linux.

Comment 7

[David Teller [:Yoric] - still alive but not very active]

Attachment: Loading symbols lazily, v4

This time, with the patch.
Also, fixed typos that caused Windows tests to fail.
Try: https://tbpl.mozilla.org/?tree=Try&rev=f533d4590e92

Comment 8

[David Teller [:Yoric] - still alive but not very active]

Attachment: Loading symbols lazily, v5

Once more, with feeling.
Try: https://tbpl.mozilla.org/?tree=Try&rev=5a64dfb442fa

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b9c7369eb615

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b9c7369eb615