Closed Bug 1560038 Opened 2 years ago Closed 1 year ago

Replace Fluent.jsm with fluent-rs in Gecko

Categories

(Core :: Internationalization, task, P2)

Product:
Core
Component:
Internationalization
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla76

People

(Reporter: zbraniecki, Assigned: zbraniecki)

References

(Blocks 2 open bugs)

Details

(Keywords: perf-alert)

Attachments

(11 files, 8 obsolete files)

Bug 1560038 - Add FluentResource.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1560038 - Add FluentMessage and FluentPattern.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1560038 - Add FluentBundle.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1560038 - Fix tests in preparation for the swap to fluent-rs.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1560038 - Add NumberFormat bindgen for Fluent.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1560038 - Switch uses of FluentBundle to use fluent-rs.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1560038 - Switch Localization IDL to use UTF8String for L10nArgs
47 bytes, text/x-phabricator-request
Details | Review
Bug 1560038 - Add FluentDateTime.
47 bytes, text/x-phabricator-request
Details | Review
vendor-in.diff
902.81 KB, patch
Details | Diff | Splinter Review
Bug 1560038 - Align FluentDateTime skeleton construction with Intl.DateTimeFormat.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1560038 - Use f64 for storing epoch instead of usize.
47 bytes, text/x-phabricator-request
Details | Review

As part of bug 1441035, we'd like to move away from using JS based Fluent parser/resolver to use Rust backed one.

I'm going to experiment with it here, but likely will have to spin off more specific bugs for things like Localization (fluent-fallback), and L10nRegistry (l10nregistry-rs).

Assignee: nobody → gandalf
Blocks: 1441035
Status: NEW → ASSIGNED
Priority: -- → P2

Bugbug thinks this bug is a task, but please change it back in case of error.

Type: defect → task
Attachment #9074690 - Attachment description: Bug 1560038 - Add XPIDL mozIFluentResource powered by Rust. → Bug 1560038 - Add fluent-rs backed FluentBundle and FluentResource WebIDL.

Question in phabritcator for :emilio, but NI in bugzilla. Yay.

Flags: needinfo?(emilio)

Is there any chance I could get a try build or some version of your patch I could apply locally? I can apply the first two revisions, but they seem to need a rebase. If I rebase manually and try to manually patch your last revision it still doesn't apply cleanly.

Flags: needinfo?(emilio) → needinfo?(gandalf)

According to our conversation on IRC only the last revision is needed, and I was on a slightly old central so I needed a rebase to apply it.

Flags: needinfo?(gandalf) → needinfo?(emilio)

Hopefully that does the trick.

Flags: needinfo?(emilio)

It did! I incorporated it into my patchset and today added support for l10nArgs. Seems like it all starts coming together.

The next step is to add an API to handle Intl formatting using ECMA402 from Rust.

Attached file perf.js (obsolete) —

I updated the patchset today to use an res-generic branch which I hope to land into fluent-rs 0.7.

I then constructed a mock script that emulates the experience of loading preferences. It's not a real timing because a lot of the operations happen async and don't really impact the talos numbers, so treat it more as a microbenchmark.

I'm attaching the script I'm using. To test the m-c JS version, just launch it in browser console. To test Rust, comment out the ChromeUtils.import and replace the FluentResource.fromSource with new FluentResource line.

Perf numbers:
JS: 15-24ms
Rust: 1.4-1.7ms

Here are profiles for both:

  1. JS: https://perfht.ml/32haRLp
  2. Rust: https://perfht.ml/32haHDN

What's also interesting is the memory:

  1. JS: 2.04Mb
  2. Rust: 606Kb

:emilio - would you have a moment to take a look at the Rust profile and check if it looks reasonable?
I plan to continue working on my patch (it's the https://phabricator.services.mozilla.com/D36286 ) but knowing if I'm doing some major mistakes at this point on the FFI boundry (by doing unnecessary allocations etc.) would be helpful.

Flags: needinfo?(emilio)

Commented on the patch.

Flags: needinfo?(emilio)
Depends on: 1449861
Attachment #9072728 - Attachment is obsolete: true
Attachment #9072729 - Attachment is obsolete: true
Attachment #9075418 - Attachment is obsolete: true

:emilio, :smaug - I need a bit of help here.

I need to add an opaque object named FluentPattern that gets returned from FluentBundle::GetMessage as part of FluentMessage shape, and then can be passed to FluentBundle::FormatPattern.

I have a patch that applies on central, that does almost that, but in FluentBundle.webidl, when I switch one line:

diff --git a/dom/chrome-webidl/FluentBundle.webidl b/dom/chrome-webidl/FluentBundle.webidl
--- a/dom/chrome-webidl/FluentBundle.webidl
+++ b/dom/chrome-webidl/FluentBundle.webidl
@@ -21,8 +21,8 @@ dictionary AttributeNamePattern {
 };
 
 dictionary FluentMessage {
-  boolean value;
-  // FluentPattern? value = null;
+  //boolean value;
+  FluentPattern? value = null;
   // sequence<AttributeNamePattern>? attributes = null;
 };

my build errors with pretty confusing messages: https://paste.mozilla.org/124qaUcW

It seems to start claiming that FluentMessage doesn't exist and OwningStringOrDouble is undefined.

I suspect that there's some problem with my opaque interface vs. dictionary in IDL because I can easily make my methods return FluentPattern, and this only happens when I try to make FluentPattern a field on the dictionary, but the debug log is unhelpful.

Can you help me pls?

Flags: needinfo?(emilio)
Flags: needinfo?(bugs)

You seem to be missing an include or a forward declaration. The WebIDL change probably just changed the order of includes.

Flags: needinfo?(emilio)

You seem to be missing an include or a forward declaration. The WebIDL change probably just changed the order of includes.

I don't understand how it would be possible. My FluentBundle.h includes FluentBundleBinding.h which has the FluentMessage declaration (in mozilla::dom).

And FluentBundle.h has using namespace mozilla::dom; so it should recognize FluentMessage and yet it doesn't. It looks like it should work well :/

Okay, I added forward declarations:

namespace dom {
struct FluentMessage;
struct L10nMessage;
class OwningStringOrDouble;
}

into FluentBundle.h, but it feels weird that I need that since I am including the FluentBundleBinding.h and LocalizationBinding.h :/ I'll keep a note for my reviewer.

Flags: needinfo?(bugs)
Attachment #9077006 - Attachment is obsolete: true
Attached file perf.js (obsolete) —

I did some perf measurements with the fluent-rs 0.7 which I released today.

I also added resolver measurements for using the bundle-writer branch which I'm considering for 0.8 [0].

m-c:

perf(parse): 14.223429999998189
perf(parse): 14.284325000000536
perf(parse): 14.540483999999196

perf(resolve): 5.433333999999377
perf(resolve): 5.521090999999615
perf(resolve): 5.435711000000083

Fluent-rs:
perf(parse): 1.3419030000004568
perf(parse): 1.4342740000010963
perf(parse): 1.4273840000005293

perf(resolve): 1.3891739999999118
perf(resolve): 1.404367999999522
perf(resolve): 1.1638170000001082

Writer:
perf(resolve): 1.0452879999998004
perf(resolve): 1.0862849999998616
perf(resolve): 1.0990079999996851

There's tons of unoptimized code still in the current PR with many unnecessary allocations, so I hope we'll get even faster. There are also ways to make parsing faster if we'll want to [1].

For now, I'll focus on completeness.

[0] https://github.com/projectfluent/fluent-rs/pull/127
[1] https://github.com/projectfluent/fluent-rs/pull/106

Depends on D54324

Attachment #9074690 - Attachment is obsolete: true

I'm going to work on extending this to support bindgen calls to ICU (for NumberFormat and DateTimeFormat), but majority of the code is in place, and I'd like to get your high-level take on the first patch in queue which adds FluentResource.

FluentResource on the JS side is going to be cached by L10nRegistry, and fed to FluentBundle in later patches. It takes a String retrieved from resource:// protocol and parses it into an AST which is stored in a rental driven struct with the String and its AST as a view into the String.

I'm interested in your feedback on the general implementation, the way it binds into WebIDL and perf/memory optimizations.

Flags: needinfo?(emilio)

So there's a ton of utf-16-to-utf-16 conversions that stands out. It'd be good if this was done in the C++ side and the rust type took a nsACString instead, probably, so it can be fixed by bug 1449861. We should definitely have a bug on file depending on that to fix that up.

Other than that the patch looks generally reasonable. I've left some comments in the first one that apply to the rest as well I think.

I can take profiles and such if you want, but it seems both perf and memory are much better than what we have in tree, so it shouldn't block anything, right?

Flags: needinfo?(emilio)

utf-16-to-utf8, that should be :)

Attachment #9110897 - Attachment description: Bug 1560038 - Add FluentResource and vendor in fluent-rs. → Bug 1560038 - Add FluentResource.
Attachment #9110930 - Attachment is obsolete: true

I still have another patch to write to use bindgen to pull:

but the rest of the code is functional and I was able to swap the JSM with Rust backend, launch Firefox, install langpack, switch locales and test pseudolocales!

Depends on: 1525869

An update on where we are:

  1. I added C++ -> Rust bindings to handle PLATFORM in line with AppConstants.platform.
  2. I added handling of basic resolver errors to be in line with Fluent.jsm
  3. We pass all intl/l10n/test and dom/l10n/test now!

What's left:

  1. Waiting for bug 1449861 and bug 1560038
  2. I need to get hooks to Intl.DateTimeFormat and Intl.NumberFormat. That will likely requite a new release of fluent-rs - https://github.com/projectfluent/fluent-rs/issues/146

I plan to work on (2) tomorrow. I don't know when (1) will get fixed, but will start poking people about the timeline (emilio and Pike respectively).

Numbers from today's m-c vs patchset:

Fluent.jsm:
parsing: [8.2, 8.4, 10.6], avg: 9.06
resolving: [4.2, 3.4, 4.3], avg: 3.96

fluent-rs:
parsing: [1.2, 1.14, 1.13], avg: 1.15 (-87.31%)
resolving: [2.4, 2.36, 2.57], avg: 2.44 (-38.38%)

Knowing that there are still a lot of improvements we can do with the rust lib (see comment 18, and the UTF8 IDL blocking bug), I expect us to be able to drive those numbers further down.

Here's the procedure I'm using now: https://gist.github.com/zbraniecki/c047476fc7435dbd0d62b04e341d56c5

Emilio: do you want to try to profile parsing/resolving using the IDL backed code?

Flags: needinfo?(emilio)
Attachment #9082464 - Attachment is obsolete: true

Profiles:

JSM parsing: https://perfht.ml/2PuwpzL
Rust parsing: https://perfht.ml/2RWQAYD
Rust resolving: https://perfht.ml/36G0OAU

Memory is similar to comment 11:

JSM: 1.55 mb
Rust: 656 kb (-57.7%)

I'm not sure I have the cycles to do that atm, but if you absolutely need to let me know and I'm happy to give it a shot.

Flags: needinfo?(emilio)

try run: https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=281647759&revision=414c0a9bf35bc712d78d7a03d4d75e95ac67e0e8

Remaining items:

  • Intl.Formatters for DATETIME() and NUMBER()
  • browser_misused_characters_in_strings.js (bug 1525869)

Nothing unexpected! Yay!

No longer blocks: 1602808

Progress - I have landed all the required work into https://github.com/projectfluent/fluent-rs master branch, and was able to get NUMBER formatting to work!

Remaining work:

  • Add more options to FluentNumberOptions
  • Add DATETIME
  • Add FluentType for DateTime
  • Make arguments of type Date passed to WebIDL be stored as FluentValue::Custom(DateTime)
Depends on: 1611506

Depends on D57402

Progress:

  • NumberFormat fully functional
  • DATETIME added
  • FluentDateTime added FluentType

Remaining work:

  • Add DateTimeOptions
  • Verify that arguments from devs passed as Date work

Progress:

  • DateTimeOptions completed (in accordance to ECMA402 rev. 4, skipped hour12)

Remaining work:

  • Fix passing date as an argument to formatPattern.

Progress:

It seems like we didn't really have a good story for the FluentDateTime in JS world either, because we collapse it as fluent-args into JSON which cannot store Date.

So, in all cases where we do use DATETIME we actually passed a number, and DATETIME accepts a number as epoch - this already works in Rust.

So, the migration will regress one unused functionality - we don't currently have FluentDateTime, FluentNumber nor FluentDateTime exposed via WebIDL. The good news is that nothing in the platform used it, and when they tried (bug 1465718) they ended up just passing a number [0].

I'd like to fix that, but it'll require some additional WebIDL/C++/Rust code, and some design work to handle non-JSON types in L10nArgs.
We could also try to attack the "store args as JSON in DOM", and implement a real native solution, but that would make Fluent drift away from being able to work as a web library.

If we stick to JSON, I'd imagine we could handle argumens as UTF8String or double or object where the object would be sth like {"type": "datetime", value: 1580112783564} or in the future some other complex type.
Either way, I'll just file a follow up.

Which means, that I believe we're complete in this bug!

I'm going now to clean up the fluent-rs crate and cut out a new release, tidy up the code in the patches we have and ask for review! If all goes well, we may be able to land it early in 75!

[0] https://searchfox.org/mozilla-central/source/devtools/client/application/src/components/service-workers/Worker.js#190

See Also: → 1611754
Depends on: 1612096
Attachment #9116281 - Attachment description: Bug 1560038 - Add bindgen for Fluent. → Bug 1560038 - Add NumberFormat bindgen for Fluent.

I think this patchset is ready for review. I've been using Firefox with it for the whole day and tested different scenarios and seems to be working well.

I'd like to target for landing this early in 75 and unblock further changes (L10nRegistry, Localization.jsm) away from JS.

See Also: → 1613271

Also, I will release fluent-bundle 0.10 before landing this. I'm awaiting a review from Stas on that.

One thing I noticed is that I seem to lost some performance win in the resolver. In comment 32 I noted ~40% win in resolver perf, but I can't reproduce it today.

I tried to remove all features I added since then (mostly formatters for number/date), and it doesn't seem to make a diff.
I tried also to compare fluent-bundle I used back then in criterion benchmark compared to the latest master against the whole about:preferences context and it seems to maintain the perf.

The only other change is the switch from DOMString to UTF8String which was intended to be faster, so I don't think it would cause the perf win loss.

:emilio - would you have time to profile the Resolving portion of https://gist.github.com/zbraniecki/c047476fc7435dbd0d62b04e341d56c5 in idlBacked = false vs idlBacked = true and verify that nothing unexpected shows up in the profile for you?

Flags: needinfo?(emilio)

Well, to be clear, UTF8String is only a perf win if you were converting to UTF-8 somewhere in the middle... So far the changes in DOMLocalization are mostly in the wrong direction (adding rather than removing conversions). Where is UTF8String used to move it to rust? Depending on the hotness of that path it may be worth doing or not.

I probably don't have time to profile myself at least this week, but if you have before / after profiles I'm happy to take a look at them or what not?

Flags: needinfo?(emilio)

Well, to be clear, UTF8String is only a perf win if you were converting to UTF-8 somewhere in the middle... So far the changes in DOMLocalization are mostly in the wrong direction (adding rather than removing conversions). Where is UTF8String used to move it to rust? Depending on the hotness of that path it may be worth doing or not.

The way it works is that you have

JS (id, args) -> C++ -> Rust -> C++ -> JS (value)
DOM (id, args) -> C++ -> Rust -> C++ -> DOM (value)

the id/args and value are all UTF8 Strings in Rust, so I assumed that it makes sense to define them as UTF8String in IDL. Otherwise I have to accept UTF16, convert to UTF8, call fluent-rs, take the result UTF8, convert to UTF16, return.

Am I wrong that using UTF8 on the WebIDL if both input and output are UTF8 is the right call?

Depends on: 1613569

With bug 1525869 in review, us not waiting for bug 1613564 and fluent-rs 0.10.2 released, I think we're nearing completion of this bug.

I updated the patchset today to use 0.10.2, and found a new incompatibility.

FluentBundle has historically been Sync+Send which is something amethyst and handlebars-fluent crates use.

As we completed the feature set to close the gap between fluent-rs and fluent.js, I introduced IntlMemoizer which is helps us manage IntlFormatters instances to be reused, rather than recreated for each bundle+options combination.

In particular, with the fluent-rs 0.10, and IntlMemoizer, I was able to introduce DateTimeFormat and NumberFormat which on the Rust side are treated like a normal IntlFormatter and are memoized by IntlMemoizer, and on the C++ side are using ICU for date and number formatting.

But here's a collision - the way we implemented those DateTimeFormat and NumberFormat FFIs of course makes them not Send+Sync friendly.
In particular, we have:

pub struct DateTimeFormat {
    raw: *mut ffi::RawDateTimeFormatter,
}

impl DateTimeFormat {
    pub fn new(locale: LanguageIdentifier, options: FluentDateTimeOptions) -> Self {
        let loc: nsCString = locale.to_string().into();
        Self {
            raw: unsafe { ffi::FluentBuiltInDateTimeFormatterCreate(&loc, &options.into()) },
        }
    }

    pub fn format(&self, input: usize) -> String {
        unsafe {
            let mut output = nsCString::new();
            ffi::FluentBuiltInDateTimeFormatterFormat(self.raw, input, &mut output);
            output.as_str_unchecked().to_owned()
        }
    }
}

impl Drop for DateTimeFormat {
    fn drop(&mut self) {
        unsafe { ffi::FluentBuiltInDateTimeFormatterDestroy(self.raw) };
    }
}

and an analogous struct for NumberFormat.

When trying to hook that into IntlMemoizer which uses Send+Sync TypeMap, I get:

 0:13.36 error[E0277]: `*mut ffi::RawDateTimeFormatter` cannot be sent between threads safely
 0:13.36    --> intl/l10n/rust/fluent-ffi/src/builtins.rs:380:14
 0:13.37     |
 0:13.37 380 |             .try_get::<DateTimeFormat>((self.options.clone(),))
 0:13.37     |              ^^^^^^^ `*mut ffi::RawDateTimeFormatter` cannot be sent between threads safely
 0:13.37     |
 0:13.37     = help: within `builtins::DateTimeFormat`, the trait `std::marker::Send` is not implemented for `*mut ffi::RawDateTimeFormatter`
 0:13.37     = note: required because it appears within the type `builtins::DateTimeFormat`

Gecko, for now, doesn't need FluentBundle to be multithreaded, so one option would be to introduce two IntlMemoizers - one with Sync+Send and another without, and then make FluentBundle take either.
Another option might be (wild guess) to implement Send+Sync for those Formatters manually and regenerate the instance and the pointer on Send. Since the instances are immutable, Sync hopefully wouldn't be an issue.

There may be other approaches, and I really don't have enough experience to decide how to tackle this.
Summoning Manish and Emilio for advise.

Flags: needinfo?(manishearth)
Flags: needinfo?(emilio)

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

With bug 1525869 in review, us not waiting for bug 1613564 and fluent-rs 0.10.2 released, I think we're nearing completion of this bug.

I updated the patchset today to use 0.10.2, and found a new incompatibility.

FluentBundle has historically been Sync+Send which is something amethyst and handlebars-fluent crates use.

Why do they use it / require it?

Another option might be (wild guess) to implement Send+Sync for those Formatters manually

Is the underlying ICU object thread-safe? If so this is just the right thing to do.

and regenerate the instance and the pointer on Send. Since the instances are immutable, Sync hopefully wouldn't be an issue.

What does this mean? I have no idea what you want to do here. Something can either be sent or not. It seems to me that as long as the ICU API is thread-safe it should be Send.

Flags: needinfo?(emilio)

(thread-safe as "movable between threads, callable from multiple threads once constructed", to be clear)

Is RawDateTimeFormatter something that is safe to be sent to another thread and accessed from there (not shared, just sent)? If so you should just unsafe impl Send forDateTimeFormat`.

Raw pointers by default aren't Send, which is the problem here.

Flags: needinfo?(manishearth)

Why do they use it / require it?

Not sure, handlebars-fluent is maintained by Manish, so maybe he can answer. Amethyst is here if you want to read the code: https://github.com/amethyst/amethyst/tree/master/amethyst_locale

Is the underlying ICU object thread-safe? If so this is just the right thing to do.

I have no experience with thread-safety so I don't know how to evaluate it.
I can point to:

What does this mean? I have no idea what you want to do here. Something can either be sent or not. It seems to me that as long as the ICU API is thread-safe it should be Send.

I guess my question is - if it is meant to be thread-safe, how do I mark it as such? If it is not, can I do sth like:

impl Send for DateTimeFormat {
    fn send(&self) -> Self {
        Self {
            loc: self.loc.clone(),
            options: self.options.clone(),
            raw: unsafe { ffi::FluentBuiltInDateTimeFormatterCreate(&self.loc, &self.options.into()) },
        }
    }
}

Is RawDateTimeFormatter something that is safe to be sent to another thread and accessed from there (not shared, just sent)? If so you should just unsafe impl Send forDateTimeFormat`.

I don't know that. I have never worked with multi-threaded code either in C++ or Rust, and the FFI was scaffolded by Emilio, so I'm also not sure how it's meant to work in the Gecko case.

How should it be designed to handle multi-thread? Or should I introduce FluentBundle with a non-multithread IntlMemoizer?

Flags: needinfo?(manishearth)
Flags: needinfo?(emilio)

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

Is the underlying ICU object thread-safe? If so this is just the right thing to do.

I have no experience with thread-safety so I don't know how to evaluate it.
I can point to:

According to http://userguide.icu-project.org/design, as long as you constrain yourself to const APIs ICU should be const-correct. Seems like DateFormat::format is const, and thus you should probably:

  • Pass a const pointer to C++ to FluentBuiltInDateTimeFormatterFormat (probably worth for the number formatter as well).
  • unsafe impl Sync for the datetime formatter.

Now regarding Send, it seems to me it should be send as long as ICU has been properly initialized.

I guess my question is - if it is meant to be thread-safe, how do I mark it as such? If it is not, can I do sth like:

  • unsafe impl Send for ... and same for sync. But please do the const pointer thing.
impl Send for DateTimeFormat {
    fn send(&self) -> Self {
        Self {
            loc: self.loc.clone(),
            options: self.options.clone(),
            raw: unsafe { ffi::FluentBuiltInDateTimeFormatterCreate(&self.loc, &self.options.into()) },
        }
    }
}

No, that's not how Send works, that doesn't make much sense to me :)

I don't know that. I have never worked with multi-threaded code either in C++ or Rust, and the FFI was scaffolded by Emilio, so I'm also not sure how it's meant to work in the Gecko case.

How should it be designed to handle multi-thread? Or should I introduce FluentBundle with a non-multithread IntlMemoizer?

It's still not 100% clear to me why the memoizer stuff needs Sync + Send, but I think per the above you should be able to make it Sync + Send.

Flags: needinfo?(emilio)

I updated the patches to add Send/Sync to DateTimeFormat and NumberFormat and am gonna test perf and memory now.

I also re-requested review on the patches that are affected.

It's still not 100% clear to me why the memoizer stuff needs Sync + Send, but I think per the above you should be able to make it Sync + Send.

Manish - ?

--

I can still introduce the non-Send/Sync IntlMemoizer as an alternative and use it in Gecko.

Flags: needinfo?(manishearth)

The patches are now back in shape and I think we're getting ready! No more hard blockers, except of maybe bug 1613564 if it lands in time.

I'm back to profile perf/mem of this change alone - we know that bug 1613705 is where majority of the talos-observable wins will come from, but my hope is that we can make this patchset already quite well optimized.

First thing, I measured parsing/resolving using https://gist.github.com/zbraniecki/c047476fc7435dbd0d62b04e341d56c5 with Firefox Profiler.
I tested on Dell XPS 7590 i9 with 64 GB RAM on Arch Linux. mozilla-central, release, from today.

I did two types of comparisons.

Variant Profile Fast Profile Detailed
Fluent.jsm https://perfht.ml/39Z1uTF https://perfht.ml/37Rrwqo
fluent-rs https://perfht.ml/38SINAO https://perfht.ml/2ViB5vN

All profiles have markers for fluent-parsing and fluent-resolving start/end.

  1. Fast

This one collects minimal amount of data, uses 1ms sampling, and is meant to keep the overhead low to give us a good approximation of the actual performance difference.

Results

Test Fluent.jsm Fluent-rs Decrease Percent
Parsing Time 18.1ms 1.4ms -16.7ms -92.26%
Parsing Memory 1980 KB 813 KB -1167 KB -59.09%
Resolve Time 11.6ms 8.7ms -2.9 ms -25.00%
Resolve Memory 500 KB 533 KB 33 KB 6.6%

The parser savings look already really nice, and with the potential additional savings in the pipeline for fluent-rs, I'm very optimistic we'll get a very good perf. In bug 1613705 I'll compare DTD/properties to full non-JS Fluent stack, but here we can definitely see a good savings on both time and memory from switching to Rust for parsing.

On the resolving front, I'm a bit concerned. In comment 18 and comment 32 I shared pretty substantial performance wins that I cannot reproduce anymore. Some of that may be due to Intl formatters initialization, but I'm worried that some of that perf has been somehow locked in some suboptimal string operation.
I'll work with the profiles to see if anything shows up.

There are still some resolver-specific opportunities - in particular the Writer trait in the resolver would allow us to pass the pre-allocated String from DOM to be populated by Rust without having to allocate strings and passing them back and forth just to append to DOM string. I hope to play with that soon, but I don't think it has to block this patchset from landing. It'll just be an implementation detail.

  1. Detailed Parser

This one collects native and js stacks and allocations and uses 0.1ms sampling to collect way more data.
I only investigated the fluent-rs profile so far.

Parser profile looks pretty decent I think. Very little JS sprinkled around, most of that there is is profiler overhead.

Out of Rust, almost all of the code is in AddResource, parsing attributes, parsing patterns and string/vec allocations.

The lexer4 branch I prototyped shows another nice win here: https://github.com/projectfluent/fluent-rs/pull/161 but except of that upcoming change, I don't expect much work to be left and I see only one further area to investigate:

6.4% is spent in FluentBundle::AddResource and half of that is hashmap and the other is malloc. This may be an area of potential optimization.

  1. Detailed Resolver

This looks way more messy.

First of all, Rust's FormatPattern which is the core of the resolver work takes only 4.8ms out of 79ms in the profile range!

Out of that, 1ms is get_message (hashbrown again!), and 1ms is resolve on an ast::Pattern! 1ms out of 79ms of resolution is actually in Rust resolving patterns :(
Another 1ms is in JS_NewStringCopyUTF8N and the rest is sprinkled around.

Ok, so what's going on in the other... 74ms?

60ms out of 78ms (77%) is in messageFromBundle, which seems reasonable. Inside that next takes 31ms (40%), values takes 8.8ms (11%) and entries takes 5.3ms (6.8%).

None of those have Rust in their stack, and Rust is separately listed under some mangled ID so I'm not sure if it's readable at all.
The stack inside all three main ones - next, values, and entries is just a lot of JS with a lot of profilers calls.

In the fluent.jsm detailed profile, the formatPattern function takes 15.3ms (out of 99ms)..

So in terms of raw JS vs Rust call to formatPattern we have 15.3ms vs. 4.8ms!

What confuses me is that I'd expect that if I call the same tests without profiler on, all of that overhead will go away, and the cost of actual calls to formatPattern should become much bigger portion of the value and the difference should be noticeable.

But if I call the testcase with profiler off, I'm still getting difference of 5.5ms to 6.6ms.

Why?

My initial hypothesis is that because we still have to call a lot of JS and garbage collection, we're not seeing this 300% win at all and only once we get rid of Localization.jsm we'll start benefiting from that part.

That would also explain why the talos doesn't show much of a win until bug 1613705 kicks in.

===========

Conclusion:

  • Land this patchset
  • Move Localization.jsm away from JS (bug 1613705)
  • Continue working on lexer for parser
  • Implement Writer pattern for DOM<-->Fluent interoper without superfluous string allocations.
  • Investigate Hashbrown use in add_messages
  • Investigate Hashbrown use in get_message

It's still not 100% clear to me why the memoizer stuff needs Sync + Send, but I think per the above you should be able to make it Sync + Send.

If you want to use the fluent stuff as a global, which is the easiest way to use it, it needs to be send/sync. The memoizer has interior mutability as a cache.

The Send+Sync case has been fixed by https://github.com/projectfluent/fluent-rs/pull/164 and as far as I'm aware this patchset is now complete.

I'm planning to land it over the next couple days.

Target Milestone: --- → mozilla76

There was a minor rounding issue in browser_html_detail_view.js which came from a bug in ICU. I fixed it by enforcing a better rounding mode.

Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7f1be4424898
Fix tests in preparation for the swap to fluent-rs. r=stas
https://hg.mozilla.org/integration/autoland/rev/e923de659c2b
Switch Localization IDL to use UTF8String for L10nArgs r=emilio
https://hg.mozilla.org/integration/autoland/rev/b4bf28bec6a8
Vendor in fluent-rs. r=fluent-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/1caf41590caf
Add FluentResource. r=emilio,smaug
https://hg.mozilla.org/integration/autoland/rev/7c69b0e5fd93
Add FluentMessage and FluentPattern. r=emilio,smaug
https://hg.mozilla.org/integration/autoland/rev/fc4da4a1c192
Add FluentBundle. r=emilio
https://hg.mozilla.org/integration/autoland/rev/570aa7aaceb5
Add NumberFormat bindgen for Fluent. r=emilio
https://hg.mozilla.org/integration/autoland/rev/56dd15fbeced
Add FluentDateTime. r=emilio
https://hg.mozilla.org/integration/autoland/rev/350df98095f8
Switch uses of FluentBundle to use fluent-rs. r=jfkthame

Backed out 9 changesets (Bug 1560038) for causing build bustages.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&collapsedPushes=659858&selectedJob=292542240&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=350df98095f8ff32bd688a59803ddb2d733483b9

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=292535660&repo=autoland&lineNumber=37006

Backout link: https://treeherder.mozilla.org/#/jobs?repo=autoland&collapsedPushes=659858&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=ddd07be487663a2f06cbc729dd565e42b5443660

[task 2020-03-10T19:59:41.992Z] 19:59:41     INFO -  Finished processing /builds/worker/workspace/obj-build/security/sandbox/linux/libmozsandbox.so in 0.20s
[task 2020-03-10T19:59:41.992Z] 19:59:41     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/security/sandbox/linux'
[task 2020-03-10T19:59:42.011Z] 19:59:42     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/toolkit/library/gtest/rust'
[task 2020-03-10T19:59:42.012Z] 19:59:42     INFO -  toolkit/library/gtest/rust/force-cargo-library-build
[task 2020-03-10T19:59:42.012Z] 19:59:42     INFO -  /builds/worker/fetches/rustc/bin/cargo rustc  --release --frozen --manifest-path /builds/worker/checkouts/gecko/toolkit/library/gtest/rust/Cargo.toml -vv --lib --target=x86_64-unknown-linux-gnu --features 'quantum_render webgpu cubeb_pulse_rust simd-accel cubeb-remoting moz_memory moz_places spidermonkey_rust cranelift_x86 gecko_profiler gecko_profiler_parse_elf new_xulstore new_cert_storage webrtc remote_agent fogotype wasm_library_sandboxing' --
[task 2020-03-10T19:59:42.012Z] 19:59:42     INFO -  error: the listed checksum of `/builds/worker/checkouts/gecko/third_party/rust/fluent-syntax/tests/fixtures/cr.ftl` has changed:
[task 2020-03-10T19:59:42.012Z] 19:59:42     INFO -  expected: 6559309b9a174ed276021eb01db80e35cab0467e30475ccb40c0cb7b24aca529
[task 2020-03-10T19:59:42.012Z] 19:59:42     INFO -  actual:   f65fd930a6aa4d9bac3cb73a220ee976161d03a57457359ee791eb733438fa5a
[task 2020-03-10T19:59:42.012Z] 19:59:42     INFO -  directory sources are not intended to be edited, if modifications are required then it is recommended that [replace] is used with a forked copy of the source
[task 2020-03-10T19:59:42.012Z] 19:59:42     INFO -  /builds/worker/checkouts/gecko/config/makefiles/rust.mk:286: recipe for target 'force-cargo-library-build' failed
[task 2020-03-10T19:59:42.013Z] 19:59:42    ERROR -  make[4]: *** [force-cargo-library-build] Error 101
[task 2020-03-10T19:59:42.013Z] 19:59:42     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/toolkit/library/gtest/rust'
[task 2020-03-10T19:59:42.014Z] 19:59:42     INFO -  /builds/worker/checkouts/gecko/config/recurse.mk:74: recipe for target 'toolkit/library/gtest/rust/target' failed
[task 2020-03-10T19:59:42.015Z] 19:59:42    ERROR -  make[3]: *** [toolkit/library/gtest/rust/target] Error 2
[task 2020-03-10T19:59:42.015Z] 19:59:42     INFO -  make[3]: *** Waiting for unfinished jobs....
[task 2020-03-10T19:59:42.069Z] 19:59:42     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/security/nss/lib/nss/nss_nss3'
[task 2020-03-10T19:59:42.069Z] 19:59:42     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang -std=gnu99 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Qunused-arguments -fcrash-diagnostics-dir=/builds/worker/artifacts -fno-strict-aliasing -ffunction-sections -fdata-sections -fno-math-errno -pthread -fPIC -pipe -g -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O2 -fno-omit-frame-pointer -funwind-tables -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments  -fPIC -shared -Wl,-z,defs -Wl,--gc-sections -Wl,-h,libnss3.so -o libnss3.so /builds/worker/workspace/obj-build/security/nss/lib/nss/nss_nss3/libnss3_so.list   -lpthread -Wl,-z,noexecstack -Wl,-z,text -Wl,-z,relro -Wl,-z,nocopyreloc -Wl,-Bsymbolic-functions -Wl,--build-id=sha1 -fstack-protector-strong -Wl,-rpath-link,/builds/worker/workspace/obj-build/dist/bin -Wl,-rpath-link,/usr/local/lib    ../../../../../config/external/nspr/pr/libnspr4.so ../../../../../config/external/nspr/libc/libplc4.so ../../../../../config/external/nspr/ds/libplds4.so ../../util/util_nssutil3/libnssutil3.so -Wl,--version-script,out.nss.def  -ldl  -lpthread -ldl -lc
[task 2020-03-10T19:59:42.069Z] 19:59:42     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/security/nss/lib/nss/nss_nss3'
[task 2020-03-10T19:59:42.185Z] 19:59:42     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/security/nss/lib/softoken/softoken_softokn3'
[task 2020-03-10T19:59:42.185Z] 19:59:42     INFO -  security/nss/lib/softoken/out.softokn.def.stub
[task 2020-03-10T19:59:42.185Z] 19:59:42     INFO -  /builds/worker/workspace/obj-build/_virtualenvs/init/bin/python -m mozbuild.action.file_generate /builds/worker/checkouts/gecko/security/generate_mapfile.py main out.softokn.def .deps/out.softokn.def.pp .deps/out.softokn.def.stub /builds/worker/checkouts/gecko/security/nss/lib/softoken/softokn.def
[task 2020-03-10T19:59:42.186Z] 19:59:42     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/security/nss/lib/softoken/softoken_softokn3'
[task 2020-03-10T19:59:42.193Z] 19:59:42     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/security/nss/lib/softoken/softoken_softokn3'
[task 2020-03-10T19:59:42.193Z] 19:59:42     INFO -  security/nss/lib/softoken/libsoftokn3.so
[task 2020-03-10T19:59:42.193Z] 19:59:42     INFO -  rm -f libsoftokn3.so
[task 2020-03-10T19:59:42.193Z] 19:59:42     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/security/nss/lib/softoken/softoken_softokn3'
[task 2020-03-10T19:59:42.250Z] 19:59:42     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/xpcom/tests'
[task 2020-03-10T19:59:42.250Z] 19:59:42     INFO -  /builds/worker/workspace/obj-build/_virtualenvs/init/bin/python -m mozbuild.action.dumpsymbols /builds/worker/workspace/obj-build/xpcom/tests/TestPRIntN /builds/worker/workspace/obj-build/xpcom/tests/TestPRIntN_syms.track
[task 2020-03-10T19:59:42.250Z] 19:59:42     INFO -  Running: /builds/worker/workspace/obj-build/_virtualenvs/init/bin/python /builds/worker/checkouts/gecko/toolkit/crashreporter/tools/symbolstore.py -c --vcs-info --install-manifest=/builds/worker/workspace/obj-build/_build_manifests/install/dist_include,/builds/worker/workspace/obj-build/dist/include -s /builds/worker/checkouts/gecko /builds/worker/workspace/obj-build/dist/host/bin/dump_syms /builds/worker/workspace/obj-build/dist/crashreporter-symbols /builds/worker/workspace/obj-build/xpcom/tests/TestPRIntN
[task 2020-03-10T19:59:42.250Z] 19:59:42     INFO -  Beginning work for file: /builds/worker/workspace/obj-build/xpcom/tests/TestPRIntN
[task 2020-03-10T19:59:42.250Z] 19:59:42     INFO -  Processing file: /builds/worker/workspace/obj-build/xpcom/tests/TestPRIntN
[task 2020-03-10T19:59:42.250Z] 19:59:42     INFO -  /builds/worker/workspace/obj-build/dist/host/bin/dump_syms /builds/worker/workspace/obj-build/xpcom/tests/TestPRIntN
[task 2020-03-10T19:59:42.250Z] 19:59:42     INFO -  Finished processing /builds/worker/workspace/obj-build/xpcom/tests/TestPRIntN in 0.11s
[task 2020-03-10T19:59:42.250Z] 19:59:42     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/xpcom/tests'
[task 2020-03-10T19:59:42.271Z] 19:59:42     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/security/nss/lib/nss/nss_nss3'
[task 2020-03-10T19:59:42.271Z] 19:59:42     INFO -  /builds/worker/workspace/obj-build/_virtualenvs/init/bin/python -m mozbuild.action.check_binary --target libnss3.so
Flags: needinfo?(gandalf)

Backed out 9 changesets (Bug 1560038) for causing build bustage

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&collapsedPushes=659858&selectedJob=292599139&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=12069dae9b8d7a05585364e58a31d884dfecd7df

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=292602539&repo=autoland&lineNumber=23462

Backout link: https://treeherder.mozilla.org/#/jobs?repo=autoland&collapsedPushes=659858&selectedJob=292599139&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=daa772c48c6e347047456be42078335cb7f531cc

[task 2020-03-11T02:53:05.214Z] 02:53:05     INFO -  /builds/worker/checkouts/gecko/config/makefiles/rust.mk:286: recipe for target 'force-cargo-library-build' failed
[task 2020-03-11T02:53:05.214Z] 02:53:05    ERROR -  make[4]: *** [force-cargo-library-build] Error 101
[task 2020-03-11T02:53:05.214Z] 02:53:05     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/toolkit/library/rust'
[task 2020-03-11T02:53:05.214Z] 02:53:05     INFO -  /builds/worker/checkouts/gecko/config/recurse.mk:74: recipe for target 'toolkit/library/rust/target' failed
[task 2020-03-11T02:53:05.214Z] 02:53:05    ERROR -  make[3]: *** [toolkit/library/rust/target] Error 2
[task 2020-03-11T02:53:05.214Z] 02:53:05     INFO -  make[3]: *** Waiting for unfinished jobs....
[task 2020-03-11T02:53:05.214Z] 02:53:05     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/toolkit/components/telemetry'
[task 2020-03-11T02:53:05.215Z] 02:53:05     INFO -  toolkit/components/telemetry/TelemetryOrigin.o
[task 2020-03-11T02:53:05.215Z] 02:53:05     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/toolkit/components/telemetry'
[task 2020-03-11T02:53:05.352Z] 02:53:05     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/toolkit/crashreporter/breakpad-client/linux'
[task 2020-03-11T02:53:05.356Z] 02:53:05     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ -std=gnu++17 -o Unified_cpp_linux0.o -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DCOMMON_LINUX_GUID_CREATOR_H__=1 -DNO_STABS_SUPPORT -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/toolkit/crashreporter/breakpad-client/linux -I/builds/worker/workspace/obj-build/toolkit/crashreporter/breakpad-client/linux -I/builds/worker/checkouts/gecko/toolkit/crashreporter/breakpad-client -I/builds/worker/checkouts/gecko/toolkit/crashreporter/google-breakpad/src -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fcrash-diagnostics-dir=/builds/worker/artifacts -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O2 -fno-omit-frame-pointer -funwind-tables -Werror -Wno-unused-local-typedefs -Wno-shadow -Wno-deprecated-declarations -Wno-bool-compare -Wno-unused-but-set-variable -Wno-c++11-narrowing -Wno-implicit-fallthrough  -MD -MP -MF .deps/Unified_cpp_linux0.o.pp   Unified_cpp_linux0.cpp
[task 2020-03-11T02:53:05.356Z] 02:53:05     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/toolkit/crashreporter/breakpad-client/linux'
[task 2020-03-11T02:53:05.356Z] 02:53:05     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/toolkit/components/telemetry'
[task 2020-03-11T02:53:05.356Z] 02:53:05     INFO -  toolkit/components/telemetry/TelemetryScalar.o
[task 2020-03-11T02:53:05.356Z] 02:53:05     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/toolkit/components/telemetry'
[task 2020-03-11T02:53:05.544Z] 02:53:05     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/toolkit/components/telemetry'
[task 2020-03-11T02:53:05.544Z] 02:53:05     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ -std=gnu++17 -o TelemetryOrigin.o -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 '-DMOZ_APP_VERSION="76.0a1"' -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/toolkit/components/telemetry -I/builds/worker/workspace/obj-build/toolkit/components/telemetry -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/checkouts/gecko/ipc/glue -I/builds/worker/checkouts/gecko/xpcom/build -I/builds/worker/checkouts/gecko/xpcom/threads -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fcrash-diagnostics-dir=/builds/worker/artifacts -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O2 -fno-omit-frame-pointer -funwind-tables -Werror -Wno-error=shadow  -MD -MP -MF .deps/TelemetryOrigin.o.pp   /builds/worker/checkouts/gecko/toolkit/components/telemetry/core/TelemetryOrigin.cpp
[task 2020-03-11T02:53:05.545Z] 02:53:05     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/toolkit/components/telemetry'
[task 2020-03-11T02:53:05.545Z] 02:53:05     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/toolkit/components/telemetry'
[task 2020-03-11T02:53:05.545Z] 02:53:05     INFO -  toolkit/components/telemetry/TelemetryIPC.o
[task 2020-03-11T02:53:05.545Z] 02:53:05     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/toolkit/components/telemetry'
[task 2020-03-11T02:53:06.487Z] 02:53:06     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/toolkit/components/printingui/ipc'
[task 2020-03-11T02:53:06.487Z] 02:53:06     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ -std=gnu++17 -o Unified_cpp_printingui_ipc0.o -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/toolkit/components/printingui/ipc -I/builds/worker/workspace/obj-build/toolkit/components/printingui/ipc -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/checkouts/gecko/ipc/glue -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fcrash-diagnostics-dir=/builds/worker/artifacts -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O2 -fno-omit-frame-pointer -funwind-tables -Werror  -MD -MP -MF .deps/Unified_cpp_printingui_ipc0.o.pp   Unified_cpp_printingui_ipc0.cpp
[task 2020-03-11T02:53:06.487Z] 02:53:06     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/toolkit/components/printingui/ipc'
[task 2020-03-11T02:53:06.487Z] 02:53:06     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/toolkit/components/telemetry'
[task 2020-03-11T02:53:06.487Z] 02:53:06     INFO -  toolkit/components/telemetry/TelemetryIPCAccumulator.o
[task 2020-03-11T02:53:06.487Z] 02:53:06     INFO -  make[4]: Leaving directory '/builds/worker/workspace/obj-build/toolkit/components/telemetry'
[task 2020-03-11T02:53:06.931Z] 02:53:06     INFO -  make[4]: Entering directory '/builds/worker/workspace/obj-build/toolkit/components/extensions/webrequest'
[task 2020-03-11T02:53:06.934Z] 02:53:06     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ -std=gnu++17 -o Unified_cpp_webrequest0.o -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/toolkit/components/extensions/webrequest -I/builds/worker/workspace/obj-build/toolkit/components/extensions/webrequest -I/builds/worker/checkouts/gecko/caps -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/checkouts/gecko/ipc/glue -I/builds/worker/checkouts/gecko/netwerk/base -I/builds/worker/checkouts/gecko/netwerk/protocol/http -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Wunused-function -Wunused-variable -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-error=tautological-type-limit-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=backend-plugin -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fcrash-diagnostics-dir=/builds/worker/artifacts -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O2 -fno-omit-frame-pointer -funwind-tables -Werror  -MD -MP -MF .deps/Unified_cpp_webrequest0.o.pp   Unified_cpp_webrequest0.cpp
[task 2020-03-11T02:53:06.935Z] 02:53:06     INFO -  In file included from Unified_cpp_webrequest0.cpp:2:
[task 2020-03-11T02:53:06.935Z] 02:53:06  WARNING -  /builds/worker/checkouts/gecko/toolkit/components/extensions/webrequest/ChannelWrapper.cpp:477:25: warning: nsIPrincipal->GetURI is depricated and will be removed soon. Please consider using the new helper functions of nsIPrincipal
[task 2020-03-11T02:53:06.935Z] 02:53:06     INFO -          Unused << prin->GetURI(getter_AddRefs(uri));
Attached patch vendor-in.diff — — Splinter Review
Flags: needinfo?(gandalf)
Backout by dluca@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c20b089e9f6e
Backed out changeset 3acb7c2cdc30 for build bustages
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c936d72fa308
Fix tests in preparation for the swap to fluent-rs. r=stas
https://hg.mozilla.org/integration/autoland/rev/55fe6b0682f4
Switch Localization IDL to use UTF8String for L10nArgs r=emilio
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/396cfd215808
Switch uses of FluentBundle to use fluent-rs. r=jfkthame
Backout by malexandru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/07c3e069a5c9
Backed out changeset 396cfd215808 for causing dt failures in browser_application_panel_list-single-worker.js

Ah, we reverted several of the date time skeleton constructions compared to LDML syntax.
Fortunately, I can just use SpiderMonkey DateTimeFormat skeleton builder for reference!

Flags: needinfo?(gandalf)
Regressions: 1622227
Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/54dbe282c9ac
Align FluentDateTime skeleton construction with Intl.DateTimeFormat. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/7cd88f40fc85
Switch uses of FluentBundle to use fluent-rs. r=jfkthame
Flags: needinfo?(gandalf)

Ok, so this is weird.

It seems to be platform specific (Windows only) and in particular, Windows 7 only.

I instrumented a build https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=293136478&revision=01e256a9dd7cc465cef423b99a01ab4ce410a5e4

in the test I did:

  const updatedEl = workerContainer.querySelector(".js-sw-updated");
  console.log(`TextContent: ${updatedEl.textContent}`);
  console.log(`Date: ${new Date().getFullYear()}`);
  ok(
    updatedEl.textContent.includes(`${new Date().getFullYear()}`),
    "Service worker has a last updated time"
  );

and got:

TextContent: January 1, 1970, 12:00:00 AM
Date: 2020

For some reason on Windows 7 and Windows 7 only the date is 0.

My first hypothesis is that it may be that somehow we cast epoch to 0? Maybe 64bit->32bit?

I'll investigate.

Flags: needinfo?(gandalf)

epoch is stored as double in ICU. Storing it as usize caused overflow on 32-bit Windows 7
which resulted in dates being formatted to epoch=0.
This patch switches to use f64 on the Rust side.

Depends on D66504

Pushed by zbraniecki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9dd488ce8dcf
Align FluentDateTime skeleton construction with Intl.DateTimeFormat. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/bdaa29b4d55d
Use i64 for storing epoch instead of usize. r=emilio
https://hg.mozilla.org/integration/autoland/rev/7eedc316b2c7
Switch uses of FluentBundle to use fluent-rs. r=jfkthame
Attachment #9133396 - Attachment description: Bug 1560038 - Use i64 for storing epoch instead of usize. → Bug 1560038 - Use f64 for storing epoch instead of usize.

Thank you everyone!

\o/

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Regressions: 1623778

:bebe, which perf alert is associated with this bug?

Flags: needinfo?(fstrugariu)

My bad wrong bug

Flags: needinfo?(fstrugariu)
Keywords: perf-alert
Attachment #9115019 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.