As part of bug 1660392 we need a more defined API for providing file I/O to L10nRegistry.

The way it is meant to work is that by default Gecko will generate FileSources and use the default FileFetcher for them, but you can override that by something like this:

let mock_ff = new FileFetcher(
  async (path) => { return "string"; },
  (path) => { return "string"; },
);

let fs1 = new FileSource(
 "mock_name",
 ["pl", "en-US"],
 "firefox-langpack-pl-en-US://localization/{locale}/",
 mock_ff)
);

and this way this FIleSource will use the mock_ff for all I/O operations. If the last argument is not provided, then the default Gecko FileFetcher is going to be used.

This will replace the current model in which people override L10nRegistry.load and L10nRegistry.loadSync for their purposes.

Actually, we can do a simpler trick without having to handle async functions back and forth:

let mock_fs = new L10nFileSource(["en-US"], "pre_path");
mock_fs.set_cache({
  "path1.ftl" : "key = Value"
});

let mockReg = new L10nRegistry();
mockReg.registerSource(mock_fs);

function generateBundles(resIds) {
  let myLocales = [];
  return mockReg.generateBundles(myLocales, resIds);
}

let loc = new Localization(, {generateBundles});

Or maybe even better, we could replace the generateBundles callback with some object that has { registry, locales[] }.

This is a micro benchmark for JS vs Rust FileSource performance.

Initial results:

Depends on D103002

Applied reviewers feedback. Current numbers:

And here are profiles created with sampling interval 0.1ms using the attached test with (true, true) for sync, and (true, false) for async:

Sync Profile:

• https://share.firefox.dev/3tKXHDU
• https://share.firefox.dev/3p54Qf4
• https://share.firefox.dev/3p6T2Zu

Async Profile:

• https://share.firefox.dev/3jAokaa
• https://share.firefox.dev/2Z0P1eD
• https://share.firefox.dev/2OqUk5f

:nika, :emilio - I believe this part is mostly stable now. I still need to run a full try build, but it passes all intl specific tests and fits into the whole patchset!

I think the only remaining part that may have a bigger impact is whether the I/O, which is 90% of what this PR does, is handling sync/async properly.
I shared the profiles above, can you take a look at them and verify that it seems sane and doesn't have unnecessary allocations/cloning?

This is going to be on a hot path for the startup so would like to use this opportunity to make sure that the I/O here is as lean as possible.

The patchset also builds against m-c and you can run the attached test in browser console to get your own profiling with markers if you want to.

I don't see anything too terrible in that profile, but I just took a quick look. malloc() shows up a bunch in the syntax parser and such, but that seems mostly unrelated to this PR. Could probably be optimized.

I'm running try runs against this part to make sure that we can land it in isolation.

I noticed that there's a difference in async loadFile behavior between JSM FileSource and Rust FileSource, likely due to some combination of microtasks/tasks.

You can reproduce it by building just D103002 PR and executing the following in the browser console:

{
const jsmReg = ChromeUtils.import("resource://gre/modules/L10nRegistry.jsm");

const fs = new jsmReg.FileSource("test", ["en-US"], "resource://app/localization/{locale}/");
const fs2 = new L10nFileSource("test", ["en-US"], "resource://app/localization/{locale}/");

async function test(fs) {
  let counter = 0;
  
  async function resolve(fs) {
    await fs.fetchFile("en-US", "browser/preferences/preferences.ftl");
    console.log(`resolve ${counter++}`);
  }

  console.log("before");
  await resolve(fs);
  resolve(fs);
  await new Promise((resolve) => resolve());
  console.log("after");
}

async function run() {
  console.log(`--- START ---`);
  await test(fs);

  console.log(`--- BETWEEN ---`);


  await test(fs2);

  console.log(`--- END ---`);
}

run();
}

The JSM version will return:

before
resolve 0
resolve 1
after

The Rust version will return:

before
resolve 0
after
resolve 1

There's of course a number of tests that rely on the JSM behavior, and I'm happy to fix them so that they actually work with async as it is intended to be.
But my concern is that maybe it is an indication of some potential performance loss between JSM and Rust due to getting out of microtask and spinning a new task, and if both behaviors are "acceptable", but the JSM's one is cheaper/faster, it's worth trying to get the Rust handling of tasks closer to what JSM is doing?

Except of the timing question and resulting test failures, the try is green! https://treeherder.mozilla.org/jobs?repo=try&revision=7ba982a1f23b30d544f2c3779fd30f343a8f9ad4

I'll wait for feedback on the microtask/task in case there is a potential for performance optimizations, and I'll keep the r? on Nika in hopes of potential guidance that I can apply to the follow up patches.

I think the tests should probably be fixed regardless, if they're relying on an async op to happen effectively sync.

But yeah I guess the way to make the behavior match would be to resolve immediately the promise in fetch_file, rather than going through moz_task. If that's easy to do, seems worth it?

Excellent! Thank you! I was able to simplify the fetch_file quite a bit and improve performance of fetching source that is already cached by ~50% by skipping the micro task!
I also fixed the test!

I pushed the whole patchset to try: https://treeherder.mozilla.org/jobs?repo=try&revision=ad8a1f9942ca4f4a25cd817e60e5aca4fa0f025a

Depends on D103002

Depends on D103259

Depends on D105392

Depends on D105393

(In reply to Zibi Braniecki [:zbraniecki][:gandalf] from comment #8)

Applied reviewers feedback. Current numbers:

Variant	JS	Rust
Sync	2.9ms	2.0ms
Async	11ms	6.7ms

And here are profiles created with sampling interval 0.1ms using the attached test with (true, true) for sync, and (true, false) for async:

Sync Profile:

• https://share.firefox.dev/3tKXHDU
• https://share.firefox.dev/3p54Qf4
• https://share.firefox.dev/3p6T2Zu

Async Profile:

• https://share.firefox.dev/3jAokaa
• https://share.firefox.dev/2Z0P1eD
• https://share.firefox.dev/2OqUk5f

:nika, :emilio - I believe this part is mostly stable now. I still need to run a full try build, but it passes all intl specific tests and fits into the whole patchset!

I think the only remaining part that may have a bigger impact is whether the I/O, which is 90% of what this PR does, is handling sync/async properly.
I shared the profiles above, can you take a look at them and verify that it seems sane and doesn't have unnecessary allocations/cloning?

This is going to be on a hot path for the startup so would like to use this opportunity to make sure that the I/O here is as lean as possible.

The patchset also builds against m-c and you can run the attached test in browser console to get your own profiling with markers if you want to.

I've took a quick look at the profiles. IMO, there is nothing to be concerned.
Please let me know if you need necko team to take a further look. Thanks.

Retesting against m-c:

Green try: https://treeherder.mozilla.org/jobs?repo=try&revision=6782b6df2020d457fa4780c717badc5e9ac1e986

Green try with full test coverage on linux opt/debug - https://treeherder.mozilla.org/jobs?repo=try&revision=65bdb1f374787b762e31dc0707a901cdb1860be9&selectedTaskRun=OwcKUxM_SV-ZsgQ2PARNnA.0

All oranges seems to be known intermittents.

Updated try: https://treeherder.mozilla.org/jobs?repo=try&revision=a8f60f13b1aefbb3aa86b925404b2242d72105b4

I consider this patchset ready to land and plan to land today.

Backed out 9 changesets (Bug 1672317) for causing toolchain bustages

Backed out for causing: https://treeherder.mozilla.org/logviewer?job_id=346788866&repo=autoland

Backout: https://hg.mozilla.org/integration/autoland/rev/3a4241c27ed786c93222bbf096414f51aadafcf6

https://hg.mozilla.org/mozilla-central/rev/13ac04e00f52
https://hg.mozilla.org/mozilla-central/rev/efeead8f81e5
https://hg.mozilla.org/mozilla-central/rev/9435cf572c9a
https://hg.mozilla.org/mozilla-central/rev/894fcec565df
https://hg.mozilla.org/mozilla-central/rev/5bb93c95e982
https://hg.mozilla.org/mozilla-central/rev/532d547483d5
https://hg.mozilla.org/mozilla-central/rev/59dde867351d
https://hg.mozilla.org/mozilla-central/rev/0af27e343a57

(In reply to Pulsebot from comment #27)

Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0af27e343a57
[l10nfilesource] part8: Remove JSM FileSource.
r=platform-i18n-reviewers,nordzilla

== Change summary for alert #30776 (as of Wed, 04 Aug 2021 10:41:54 GMT) ==

Regressions:

Improvements:

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=30776