mach logspam

[Eric Rahm]

Hi folks-

After a long delay I've finally integrated my logspam tools
 into mach
. For reference, see
some past discussions on the War on Warnings
.

*What is logspam  you
ask?*
It's the warning spew we see when running debug builds, particularly during
testing. Sometimes these warnings are useful and point out a real problem,
sometimes they're just useful when initially developing a feature to track
program flow but lose utility once the feature lands. Generally the sheer
volume just makes it hard to find any real issues.

*What does the tool do?*

   - *Reporting* - Downloads and parses test results to present a summary
   of the most common warnings
   - *Bisecting* - Helps narrow down when a warning showed up
   - *Filing* - Automates the process of filing a detailed bug in the
   appropriate component

*How do I use it?*
Examples and extended help are available in the github repo
 as well as via the
command line
mach logspam --help

A few quick examples:

   - Report on the latest linux build on central:
   ./mach logspam report latest
   - Report on try run:
   ./mach logspam report --repo try 2f23de3f6837d7fa7102fd8affc65f1d296f9e11
   - Specific details of a warning found in that try run including which
   test suites and tests it shows up in:
   ./mach logspam report --repo try \
   2f23de3f6837d7fa7102fd8affc65f1d296f9e11 \
   "WARNING: Shouldn't call SchedulePaint in a detached pres context:
   file layout/generic/nsIFrame.cpp, line 6868"

*What's next?*
If folks want to improve the tool I'm happy to land patches in my repo, or
if someone wants to support it officially in the mozilla repo I'm happy to
transfer ownership.

-e
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to Implement and Ship: Make MOZ_QUIET the default, require opt-in for DOMWINDOW/DOCSHELL logs

[Eric Rahm]

This sounds great. Is there a reason we can't just use MOZ_LOG for the
docshell/domwindow logging?

-e


On Fri, Nov 8, 2019 at 2:44 PM Tom Ritter  wrote:

> In https://bugzilla.mozilla.org/show_bug.cgi?id=1592297 I plan/hope to
> remove MOZ_QUIET and turn off the DOCSHELL/DOMWINDOW logging by default.
> It will automatically be enabled in browser-chrome tests where it is
> needed. (It actually will no longer be possible to disable it when running
> those tests also.)
>
> Once this lands, MOZ_QUIET will no longer do anything, and if you want this
> output you will need to explicitly set a new env var.  (Current idea is
> MOZ_LOG_WINDOWS_DOCSHELLS but feel free to tell me what color for the
> bikeshed in phabricator, I have no preference.)
>
> I'm sending this before I finish the requested changes in phabricator to
> give folks an opportunity to raise objections; I hope to land it mid/late
> next week.
>
> -tom
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Must we rebuild all our rust code constantly?

[Eric Rahm]

A fix for this (https://bugzilla.mozilla.org/show_bug.cgi?id=1576030)
should be on m-c shortly, no need to tweak mozconfigs.

-e

On Fri, Aug 23, 2019 at 9:45 AM Nicholas Alexander 
wrote:

> On Fri, Aug 23, 2019 at 2:11 AM ishikawa  wrote:
>
> > On 2019/08/23 11:00, Mike Hommey wrote:
> > >
> > > In the meanwhile, as discussed on irc, it seems fair to disable
> > > CARGO_INCREMENTAL when building with sccache.
> >
> > So how can we disable this?
> > Setting environment variable as in the following?
> >
> > export CARGO_INCREMENTAL=
> >
>
> `ac_add_options --disable-cargo-incremental` in your mozconfig should do
> it.  See:
>
>
> https://searchfox.org/mozilla-central/rev/597a69c70a5cce6f42f159eb54ad1ef6745f5432/build/moz.configure/toolchain.configure#1865
>
> Nick
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Must we rebuild all our rust code constantly?

[Eric Rahm]

You can use |mach clobber --full| to remove the incremental cache. It might
be worth filing a bug for the large size, my best guess is that we're
keeping around items from old compiler versions. It's possible we could do
a full clobber of the IC when we detect a new compiler version, but that
might not be a great experience for folks testing on different rust
versions.

-e

On Tue, Aug 20, 2019 at 11:02 AM ISHIKAWA,chiaki 
wrote:

> On 2019/08/20 14:32, Kris Maglione wrote:
> > On Tue, Aug 20, 2019 at 02:23:06PM +0900, ISHIKAWA,chiaki wrote:
> >> On 2019/08/20 9:11, Dave Townsend wrote:
> >>> Thanks to a tip I've tracked this down. This seems to only be the
> >>> case when
> >>> I have sccache enabled. Disabling it gives me nice quick incremental
> >>> builds
> >>> again. Of course that isn't an ideal solution but it will do for now.
> >>>
> >>> On Mon, Aug 19, 2019 at 1:55 PM Dave Townsend
> >>>  wrote:
> >>>
>  For a couple of weeks now I've seen that any attempt to build Firefox,
>  even incremental builds seem to rebuild an awful lot of rust code.
>  I found
>  this in the source which seems to suggest why:
> 
> https://searchfox.org/mozilla-central/source/config/makefiles/rust.mk#238.
>
> 
>  But, this means that now an incremental build with a couple of code
>  change
>  that have no impact on rust is taking upwards of 4 minutes to
>  complete in
>  comparison to around 40 seconds, and the log file is full of cargo
>  output.
>  I've heard similar comments from other developers.
> 
>  This is a pretty big increase in the time to compile and test and is
>  really slowing down my work. Is there any way we can avoid this?
> 
> >>> 
> >>
> >>
> >> I am using linux for local development and noticed something similar.
> >>
> >> So I should disable sccache (!?).
> >
> > For what it's worth, Rust is now configured to use incremental
> > compilation, which has its own cache which isn't cleared after
> > clobber, so sccache isn't actually needed anymore. Ordinary ccache
> > should be sufficient.
> >
> >
>
> I was not sure where this incremental compilation cache is located.
> Sure enough on my computer it seems to live under
> MOZ_OBJ/x86_64-unknown-linux-gnu/debug/incremental
>
> I have a serious question: does this incremental cache have a control
> mechanism to purge old entries?
>
> I have been hit with file system overflow errors quite frequently since
> early summer and I figure
> this is due to the bloated
> MOZ_OBJ/x86_64-unknown-linux-gnu/debug/incremental directory.
>
> It is now 26.6 GB with 67840 items according baobab file system usage
> monitor.
>
> 26.6 GB for this incremental cache alone is way too much number that was
> suggested by mozilla development document before.
> In comparsion, ccache I use is usable with max size of 5GB
> as set by
> CCACHE_SIZE=5.0GB
> in my script.
>
> On my local PC, I see that the file directory size (with the sizes of
> subdirectories added to it) is as follows:
>
> MOZ_OBJ is 45.8 GB large. (as of now: I had to stop TB compilation due
> to the 50 GB smallish partition overflow where this directory is located.)
> MOZ_OBJ/x86_64-unknown-linux-gnu/ is 33.9 GB large .
> MOZ_OBJ/x86_64-unknown-linux-gnu/debug/  is 33.9 GB large.
>  I think /debug/ is because I create full debug version of TB locally.
> MOZ_OBJ/MOZ_OBJ/x86_64-unknown-linux-gnu/debug/incremental is 26.5 GB
> large.
>
> I need a way to shrink this incremental cache reasonably.
> It seems this incremental directory grows without limit. Or, it does not
> trim old unusable cache files automatically.
>
> TIA
>
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Structured bindings and minimum GCC & clang versions

[Eric Rahm]

We are actively working on this. Unfortunately, as expected, its never as
simple as we'd like. Updating the minimum gcc version (
https://bugzilla.mozilla.org/show_bug.cgi?id=1536848) is blocked on getting
our hazard builds updated, updating to c++17 has some of it's own quirks.
We're currently at the point where doing a try build w/ c++17 enabled on
linux works.


On Thu, Aug 15, 2019 at 11:45 PM Gerald Squelart 
wrote:

> On Friday, August 16, 2019 at 4:19:20 PM UTC+10, Henri Sivonen wrote:
> > This week, I wrote some code that made me wish we already had support
> > for structured bindings and return by initializer list (both from
> > C++17) for mozilla::Tuple.
> >
> > That is, if we have
> > mozilla::Tuple Foo()
> > it would be nice to be able to call it via
> > auto [a, b] = Foo();
> > and within Foo to write returns as
> > return { a, b };
> >
> > It appears that our minimum GCC and minimum clang documented at
> >
> https://developer.mozilla.org/en-US/docs/Mozilla/Using_CXX_in_Mozilla_code
> > are pretty old.
> >
> > What's the current outlook for increasing the minimum GCC and clang
> > versions such that we could start using structured bindings and return
> > by initializer list for tuples (either by making sure mozilla::Tuple
> > support these or by migrating from mozilla::Tuple to std::tuple) and
> > thereby get ergonomic multiple return values in C++?
> >
> > --
> > Henri Sivonen
> > hsi...@mozilla.com
>
> I'm guessing that https://bugzilla.mozilla.org/show_bug.cgi?id=1560664
> will deal with compiler details. (I don't think we can use a C++17 feature
> without enabling C++17 overall -- right?)
>
> I too am eager to start using C++17, mostly constexpr-if.
>
> Cheers,
> Gerald
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Proposal to adjust testing to run on PGO builds only and not test on OPT builds

[Eric Rahm]

Just a heads up, it looks like this landed sometime last week for platforms
that support PGO.

This has an unintended consequence of making it look like perf data for
integration branches went awol, but in fact you need to switch from looking
at "opt" data to "pgo" data. Unfortunately since we didn't used to run PGO
on integration branches you'll also need to include opt data in your view
for continuity. For example I've resorted to using an opt/pgo mashup to
track memory regressions [1]. This seems to work okay for memory, but I can
imagine it won't be a fair comparison for talos tests.

-e

[1]
https://treeherder.mozilla.org/perf.html#/graphs?series=mozilla-central,1888424,1,4=mozilla-inbound,1890241,1,4=autoland,1885459,1,4=mozilla-central,1684866,1,4=mozilla-inbound,1684808,1,4=autoland,1684872,1,4

On Mon, Jan 21, 2019 at 2:38 AM James Graham  wrote:

> On 21/01/2019 10:18, Jan de Mooij wrote:
> > On Fri, Jan 18, 2019 at 10:36 PM Joel Maher 
> wrote:
> >
> >> Are there any concerns with this latest proposal?
> >>
> >
> > This proposal sounds great to me. Thank you!
>
> +1. This seems like the right first step to me.
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Fission MemShrink Newsletter #2

[Eric Rahm]

Hi Folks!

The Fission MemShrink project has been chugging along quite nicely and a
ton of progress has been made over the past month. There's been a large
focus on reducing the JS memory usage and we've managed to drop the base
content JS measure
<https://treeherder.mozilla.org/perf.html#/graphs?timerange=2592000=autoland,1684872,1,4>
by ~1.1MB (17%) [1]:

[image:
https://screenshotscdn.firefoxusercontent.com/images/c5046573-91bd-4ca1-98db-6290777e70c4.png]

Additionally we've started looking at the overhead of our many threads
<https://bugzilla.mozilla.org/show_bug.cgi?id=1476432> [2], drafted up some
ideas on reducing relocation overhead by adopting a fork server
<https://bugzilla.mozilla.org/show_bug.cgi?id=1470591> on linux/mac [3],
and we're looking at reducing the overhead of per process font related data
<https://bugzilla.mozilla.org/show_bug.cgi?id=648417> [4].

Below is a sampling of the bugs that have improved memory usage over the
past month, apologies if I left any off! Please feel free to point out
other improvements. All numbers are *per content process*.

*JS improvements*

   - In a great example of every little bit counts, *Kris Maglione* reduced
   memory usage by *405KB* in various bugs that made us lazily load JS
   content (Bug 1472491
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1472491>, Bug 1473631
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1473631>, Bug 1474139
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1474139>, Bug 1474155
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1474155>, Bug 1479241
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1479241>, Bug 1479245
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1479245>, Bug 1479309
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1479309>, Bug 1479310
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1479310>, Bug 1479312
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1479312>, Bug 1479313
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1479313>, Bug 1479318
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1479318>, Bug 1480319
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1480319>, Bug 1480327
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1480327>, Bug 1483363
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1483363>)
   - Kris also reduced memory usage by *494KB* by getting rid of
   MessageManager globals (Bug 1480244
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1480244>)
   - *Felipe Gomes* reduced memory usage by *95KB* by lazily loading JS
   content (Bug 1369466
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1369466>, Bug 1470324
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1470324>)
   - *Mike Kaply* reduced memory usage by *65KB* by converting an add-on to
   in tree telemetry (Bug 1475571
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1475571>)
   - *Dão Gottwald* reduced memory uasge by *57KB* by lazily generating
   thumbnails for the Ctrl+Tab panel (Bug 1481321
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1481321>)
   - *Nick Nethercote* reduced memory usage by *44KB* by making
   mozilla::HashMaps lazily allocate their storage (Bug 1481998
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1481998>)
   - *Benjamin Bouvier* reduced memory usage by *25KB* by removing SIMD.js (Bug
   1416723 <https://bugzilla.mozilla.org/show_bug.cgi?id=1416723>)

*Threading Overhead*

   - Kris added measurements for thread stacks across platforms (Bug 1475899
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1475899>, Bug 1476405
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1476405>, Bug 1477512)
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1477512>
   - Kris reduced memory usage on Linux by *~3MB* (that's megabytes!) by
   changing our default stack size (Bug 1476828
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1476828>)
   - *Eric Rahm* (this author) reduced memory usage by *50KB* by reducing
   the amount of networking threads (Bug 1448034
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1448034>)
   - *Xidorn Quan* committed an upstream Rust patch (rust-lang/rust#52847
   <https://github.com/rust-lang/rust/issues/52847>) to fix how stack sizes
   are set on windows after Kris' initial investigation (Bug 1479250
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1479250>)

*Font Overhead*

   - The winner of biggest improvement with the smallest change goes to *Lee
   Salzman* who reduced memory usage on OSX by *10MB* by reducing the skia
   glyph cache (Bug 1258781
   <https://bugzilla.mozilla.org/show_bug.cgi?id=1258781>)

*Lessons learned*

Small fixes add up, removing dead code not only helps code quality but can
save memory, adding measurements is important, caches can be easy targets.
If you know of a cache we might tweak please file a bug to consider
reducing it.

-e

[1]
https:

Re: mozilla::Hash{Map,Set}

[Eric Rahm]

On Thu, Aug 16, 2018 at 6:07 AM, Alex Gaynor  wrote:

> Would it make sense to consider giving nsTHashtable and PLDHashTable
> different names? Right now the names suggest that we have 3 general purpose
> hash tables, but it might be easier if the names better suggested their
> purpose, e.g. PLDHashTable -> MinimalCodeSizeHashTable (I'm sure we can do
> better than that).
>

nsTHashTable is just a templated wrapper around PLDHashTable. For reference
we also have nsDataHashtable, nsClassHashtable, nsRefPtrHashtable,
nsInterfaceHashtable [1] all of which are rather convenient wrappers around
PLDHashTable. These have also been implemented in such a way that the
templates have lower code size overhead (thanks froydnj!). I somewhat
prefer their interfaces to mfbt::HashMap (default infallible, plenty of
predefined hashers, nice lookup for add semantics, etc), but to each their
own.

[1]
https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Guide/Hashtables



> Similarly, would it make sense to consolidate the APIs, as much as
> possible, if the primary differences are around implementation details like
> "how much is templated/inlined"?
>
>
Really we just have PLDHashTable for historical reasons, not because of
text size. I'm not sure we even want to keep PLDHashTable at all. We *have*
found some issues where there's higher memory overhead due to the way our
template wrappers pad their key structures and having a much lower level,
but slightly annoying to use hashtable ended up being a good thing.
Additionally we've been thinking about implementing a better hash algorithm
such as round-robin hashing [2] , I'm not sure if that would work w/
mfbt::HashMap, but it would be nice to implement it in one place rather
than two.

[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1402910

-e



> Alex
>
>
> On Wed, Aug 15, 2018 at 5:39 AM Nicholas Nethercote <
> n.netherc...@gmail.com>
> wrote:
>
> > Hi,
> >
> > I recently moved Spidermonkey's js::Hash{Map,Set} classes from
> > js/public/HashTable.h into mfbt/HashTable.h and renamed them as
> > mozilla::Hash{Map,Set}. They can now be used throughout Gecko.
> >
> > (js/public/HashTable.h still exists in order to provide renamings of
> > mozilla::Hash{Map,Set} as js::Hash{Map,Set}.)
> >
> > Why might you use mozilla::Hash{Map,Set} instead of PLDHashTable (or
> > nsTHashtable and other descendants)?
> >
> > - Better API: these types provide proper HashMap and HashSet instances,
> and
> > (in my opinion) are easier to use.
> >
> > - Speed: the microbenchmark in xpcom/rust/gtest/bench-
> collections/Bench.cpp
> > shows that mozilla::HashSet is 2x or more faster than PLDHashTable. E.g.
> > compare "MozHash" against "PLDHash" in this graph:
> >
> >
> > https://treeherder.mozilla.org/perf.html#/graphs?
> timerange=604800=mozilla-central,1730159,1,6&
> series=mozilla-central,1730162,1,6=mozilla-
> central,1730164,1,6=mozilla-central,1732092,1,6&
> series=mozilla-central,1730163,1,6=mozilla-central,1730160,1,6
> >
> > Bug 1477627 converted a hot hash table from PLDHashTable to
> > mozilla::HashSet and appears to have sped up cycle collection in some
> cases
> > by 7%. If you know of another PLDHashTable that is hot, it might be worth
> > converting it to mozilla::HashTable.
> >
> > Both mozilla::Hash{Map,Set} and PLDHashTable use the same double-hashing
> > algorithm; the speed difference is due to mozilla::HashSet's extensive
> use
> > of templating and inlining. The downside of this is that mozilla::HashSet
> > will increase binary size more than PLDHashTable.
> >
> > There are overview comments at the top of mfbt/HashTable.h, and the
> classes
> > themselves have more detailed comments about every method.
> >
> > Nick
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> >
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: PSA: Major preference service architecture changes inbound

[Eric Rahm]

We *could* special case prefs with an appropriate data structure that works
in a thread-safe nature; as far as RWLock's go, we do have one in tree [1].
This has gone off the rails a bit from Kris' original announcement, which
I'll reiterate: Watch out for prefs related bustage.

Jeff, would you mind filing a bug for further discussion of off-main-thread
access as a future improvement?

[1] https://searchfox.org/mozilla-central/source/xpcom/threads/RWLock.h

On Thu, Jul 19, 2018 at 7:25 PM, Kris Maglione 
wrote:

> On Thu, Jul 19, 2018 at 07:17:13PM -0700, Jeff Gilbert wrote:
>
>> Using a classic read/write exclusive lock, we would only every contend
>> on read+write or write+write, which are /rare/.
>>
>
> That might be true if we gave up on the idea of switching to Robin Hood
> hashing. But if we don't, then every lookup is a potential write, which
> means every lookup required a write lock.
>
> We also don't really have any good APIs for rwlocks at the moment. Which,
> admittedly, is a solvable problem.
>
>
> On Thu, Jul 19, 2018 at 2:19 PM, Kris Maglione 
>> wrote:
>>
>>> On Tue, Jul 17, 2018 at 03:49:41PM -0700, Jeff Gilbert wrote:
>>>

 We should totally be able to afford the very low cost of a
 rarely-contended lock. What's going on that causes uncached pref reads
 to show up so hot in profiles? Do we have a list of problematic pref
 keys?

>>>
>>>
>>> So, at the moment, we read about 10,000 preferences at startup in debug
>>> builds. That number is probably slightly lower in non-debug builds, bug
>>> we
>>> don't collect stats there. We're working on reducing that number (which
>>> is
>>> why we collect statistics in the first place), but for now, it's still
>>> quite
>>> high.
>>>
>>>
>>> As for the cost of locks... On my machine, in a tight loop, the cost of a
>>> entering and exiting MutexAutoLock is about 37ns. This is pretty close to
>>> ideal circumstances, on a single core of a very fast CPU, with very fast
>>> RAM, everything cached, and no contention. If we could extrapolate that
>>> to
>>> normal usage, it would be about a third of a ms of additional overhead
>>> for
>>> startup. I've fought hard enough for 1ms startup time improvements, but
>>> *shrug*, if it were that simple, it might be acceptable.
>>>
>>> But I have no reason to think the lock would be rarely contended. We read
>>> preferences *a lot*, and if we allowed access from background threads, I
>>> have no doubt that we would start reading them a lot from background
>>> threads
>>> in addition to reading them a lot from the main thread.
>>>
>>> And that would mean, in addition to lock contention, cache contention and
>>> potentially even NUMA issues. Those last two apply to atomic var caches
>>> too,
>>> but at least they generally apply only to the specific var caches being
>>> accessed off-thread, rather than pref look-ups in general.
>>>
>>>
>>> Maybe we could get away with it at first, as long as off-thread usage
>>> remains low. But long term, I think it would be a performance foot-gun.
>>> And,
>>> paradoxically, the less foot-gunny it is, the less useful it probably is,
>>> too. If we're only using it off-thread in a few places, and don't have to
>>> worry about contention, why are we bothering with locking and off-thread
>>> access in the first place?
>>>
>>>
>>> On Tue, Jul 17, 2018 at 8:57 AM, Kris Maglione 
 wrote:

>
> On Tue, Jul 17, 2018 at 02:06:48PM +0100, Jonathan Kew wrote:
>
>>
>>
>> On 13/07/2018 21:37, Kris Maglione wrote:
>>
>>>
>>>
>>> tl;dr: A major change to the architecture preference service has just
>>> landed, so please be on the lookout for regressions.
>>>
>>> We've been working for the last few weeks on rearchitecting the
>>> preference service to work better in our current and future
>>> multi-process
>>> configurations, and those changes have just landed in bug 1471025.
>>>
>>
>>
>>
>> Looks like a great step forward!
>>
>> While we're thinking about the prefs service, is there any possibility
>> we
>> could enable off-main-thread access to preferences?
>>
>
>
>
> I think the chances of that are pretty close to 0, but I'll defer to
> Nick.
>
> We definitely can't afford the locking overhead—preference look-ups
> already
> show up in profiles without it. And even the current limited exception
> that
> we grant Stylo while it has the main thread blocked causes problems
> (bug
> 1474789), since it makes it impossible to update statistics for those
> reads,
> or switch to Robin Hood hashing (which would make our hash tables much
> smaller and more efficient, but requires read operations to be able to
> move
> entries).
>
> I am aware that in simple cases, this can be achieved via the
>> StaticPrefsList; by defining a VARCACHE_PREF there, I can read its
>> value
>> from 

Re: Intent to remove: the 'Memory usage of Subprocesses' table from about:performance

[Eric Rahm]

Thanks Florian, considering it's roughly unmaintained right now, leaking,
and showing up in perf profiles it sounds reasonable to remove the memory
section. I've filed bug 1475301 [1] to allow us to measure USS off main
thread; we can deal with adding that back in the future if it makes sense.

-e

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1475301

On Thu, Jul 12, 2018 at 12:41 AM, Florian Quèze  wrote:

> On Thu, Jul 12, 2018 at 1:18 AM, Eric Rahm  wrote:
>
> > What performance issues are you seeing? RSS and USS should be relatively
> > lightweight and the polling frequency isn't very high.
>
> It seems ResidentUniqueDistinguishedAmount does blocking system calls,
> resulting in blocking the main thread for several seconds in the worst
> case. Here's a screenshot of a profile showing it:
> https://i.imgur.com/DjRMQtY.png (unfortunately that profile is too big
> and fails to upload with the 'share' feature).
>
> There's also a memory leak in the implementation, after leaving
> about:performance open for a couple hours, there was more than 300MB
> of JS "Function" and "Call" objects (about:memory screenshot:
> https://i.imgur.com/21YNDru.png ) and the devtools' Memory tool shows
> that this is coming from the code queuing updates to that subprocess
> memory table: https://i.imgur.com/04M71hg.png
>
> Florian
>
> --
> Florian Quèze
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Intent to remove: the 'Memory usage of Subprocesses' table from about:performance

[Eric Rahm]

This was added in bug 1255843 [1]. I don't feel to strongly about keeping
it around, I believe mconley and mrbkap came up with the idea of adding it.
It's *much* more lightweight than about:memory and provides automatic
updates which is nice for monitoring without external tools.

What performance issues are you seeing? RSS and USS should be relatively
lightweight and the polling frequency isn't very high.

-e

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1255843

On Wed, Jul 11, 2018 at 11:12 AM, Florian Quèze  wrote:

> Context: we are currently redesigning about:performance to make it
> more useful for users.
>
> This section of the current about:performance page provides
> information that isn't actionable for users, and collecting this
> information causes performance issues, so I think it's time to remove
> it.
>
> I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1474990 for the
> removal.
>
> If someone uses this information and strongly thinks we should keep it
> (eg. by moving it to about:memory), now is the time to speak up.
>
> Thanks,
> Florian
>
> --
> Florian Quèze
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Coding style: brace initialization syntax

[Eric Rahm]

Reading back through I think the consensus, at least for initializer lists
was:

   1. Prefer parenthesis, ie:
   , mBool(true)
   2. If using braces, maintain the same spacing you would use with
   parenthesis, ie:
   , mStructWithoutCtor{42}

1. was pragmatic as this is what we already do, 2. was for consistency with
1.

To answer Bogdan's question, it looks like we prefer [1], although it would
be nice to see that codified in our style doc.

jya, you make some interesting points, but we should keep the scope of this
discussion focused. You might want to raise them in separate threads --
"Should we recommend initialization at member declaration", "Should we
recommend where a ctor should is defined", etc.

-e


On Tue, Jun 5, 2018 at 5:50 AM, Jean-Yves Avenard 
wrote:

>
>
> > On 5 Jun 2018, at 12:54 pm, bposteln...@mozilla.com wrote:
> >
> > I would like to resurrect this thread since it would help us a lot for
> bug 1453795 to come up to a consensus on when to use bracelets and when to
> use parenthesis. Also I must point out a thing here, that was also
> mentioned here earlier, that there are situations where we cannot use
> parenthesis. This is when we want to initialize a structure that doesn't
> have a ctor, like:
> > [1]
> > struct str {
> >  int a;
> >  int b;
> > };
> >
> > class Str {
> >  str s;
> >  int a;
> > public:
> >  Str() : s{0}, a(0) {}
> > };
> >
> > Also it would help a lot if we would establish how many, spaces should
> be between the parenthesis or the bracelets, like how do we prefer [1] or
> [2]
> >
> > [2]
> > class Str {
> >  str s;
> >  int a;
> > public:
> >  Str() : s{ 0 }, a( 0 ) {}
> > };
> >
> > I don't have a personal preference here, but right now there are several
> places in our code that combine spaces between parenthesis/bracelets with
> no spaces.
>
> The current coding style: https://developer.mozilla.org/
> en-US/docs/Mozilla/Developer_guide/Coding_Style states to not use space.
>
> There’s no case where a parenthesis should be followed by a space.
>
> Many things wrong here:
> First the bracket { should be on a new line :
>
> class/struct str
> {
> …
> }
>
> Initialization are to be on multiple-lines.
>
> clang-format would have made it:
>   class Str
>   {
> str s;
> int a;
>
>   public:
> Str()
>   : s{ 0 }
>   , a(0)
> {
> }
>   };
>
> IMHO, should be going for C++11 initializer, it’s much clearer, and avoid
> duplicated code when you need multiple constructors.
> What is str? I assume not a plain object, so it should have its own
> initializer.
>
> so it all becomes:
>   class Str
>   {
> str s;
> int a = 0;
>
>   public:
> Str() {}
>   };
>
> or:
>   class Str
>   {
> str s;
> int a = 0;
>
>   public:
> Str() = default;
>   };
>
> (and I prefer constructors to be defined at the start of the class
> definition)
>
> My $0.02
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Is realloc() between bucket sizes worthwhile with jemalloc?

[Eric Rahm]

On Mon, Apr 9, 2018 at 4:58 AM, Henri Sivonen  wrote:

> My understanding is that under some "huge" size, jemalloc returns
> allocations from particularly-sized buckets.
>

The mozjemalloc source has a decent ascii-art table [1].


> This makes me expect that realloc() between bucket sizes is going to
> always copy the data instead of just adjusting allocated metadata,
> because to do otherwise would mess up the bucketing.
>

Sure, but not all reallocs are between bucket sizes. You can realloc from
1KB to 1.99KB and end up in the same bucket.


> Is this so? Specifically, is it actually useful that nsStringBuffer
> uses realloc() as opposed to malloc(), memcpy() with actually
> semantically filled amount and free()?
>
> Upon superficial code reading, it seems to me that currently changing
>
the capacity of an nsA[C]STring might uselessly use realloc to copy
> data that's not semantically live data from the string's point of view
> and wouldn't really need to be preserved. Have I actually discovered
> useless copying or am I misunderstanding?
>

In this case I think you're right. In the string code we use a doubling
strategy up to 8MiB so they'll always be in a new bucket/chunk. After 8MiB
we grow by 1.125 [2], but always round up to the nearest MiB. Our
HugeRealloc logic always makes a new allocation if the difference is
greater than or equal to 1MiB [3] so that's always going to get hit. I
should note that on OSX we use some sort of 'pages_copy' when the realloc
is large enough, this is probably more efficient than memcpy.

-e

[1]
https://searchfox.org/mozilla-central/rev/7ccb618f45a1398e31a086a009f87c8fd3a790b6/memory/build/mozjemalloc.cpp#59-88
[2]
https://searchfox.org/mozilla-central/rev/7ccb618f45a1398e31a086a009f87c8fd3a790b6/xpcom/string/nsTSubstring.cpp#88-119
[3]
https://searchfox.org/mozilla-central/rev/7ccb618f45a1398e31a086a009f87c8fd3a790b6/memory/build/mozjemalloc.cpp#3811-3874

>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Null[C]String() has been renamed Void[C]String()

[Eric Rahm]

The problem is these were never a null string, they're a voided empty
string. If you do a `str.get()` it gives you a valid pointer, not nullptr.
Regardless, while this was primarily a string change, we probably should
have pinged a dom peer; I had forgotten the weirdness that is DOMString
[1]. Arguably that API should be taking a maybe param and arguably we
should get rid of the void concept but that's not straightforward.

As a workaround for readability I think it would be reasonable to add some
sort of `using NullString = VoidString` to DOMString.h.

-e

[1]
http://searchfox.org/mozilla-central/rev/15ce5cb2db0c85abbabe39a962b0e697c9ef098f/dom/bindings/DOMString.h#153-173

On Fri, Sep 22, 2017 at 6:02 AM, Boris Zbarsky  wrote:

> On 9/22/17 2:41 AM, Nicholas Nethercote wrote:
>
>> This probably won't affect most people, because void strings are a niche
>> feature.
>>
>
> Not in the DOM.  They're used anytime you see "DOMString?" in webidl.
> http://searchfox.org/mozilla-central/search?q=DOMString%3F=webidl
> shows several hundred hits.
>
> Looking at the diff from bug 1401813, it looks like some of the uses are
> in fact using the XPCOM "void string" feature per se, while some are
> conceptually trying to represent "JS null" (e.g. in nullable string types
> about to be passed to JS) and for those having a concept of null string, in
> the sense defined by dom/base/nsDOMString.h, makes sense.
>
> Put another way, in the new setup the element lifecycle callbacks code in
> Element::SetAttrAndNotify/UnsetAttr is much more confusing than it used
> to be...
>
> -Boris
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Re-visiting the DOM tree depth limit in layout

[Eric Rahm]

Jan is right, increasing the stack size for all threads would cause a very
large memory regression. Lets not do that ;) Selectively increasing might
make sense but we'd need to keep an eye on how large of an increase we're
expecting.

-e

On Wed, Sep 13, 2017 at 7:19 AM, Jan de Mooij  wrote:

> On Wed, Sep 13, 2017 at 10:44 AM, Henri Sivonen 
> wrote:
>
> > 3) Increase the run-time stack size on Windows such that 1026-deep
> > display: table-cell doesn't overflow the stack.
> >
>
> On Windows, threads that are created without an explicit stack size also
> use the executable's stack size. So increasing our stack size will also
> affect a bunch of other threads and we risk an increase in virtual memory
> OOMs on Win32. I filed bug 1257522 [0] two years ago to pass an explicit
> stack size for a number of background threads we create in Gecko, it would
> be nice to fix that first.
>
> Jan
>
> [0] https://bugzilla.mozilla.org/show_bug.cgi?id=1257522
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Coding style: Placement of binary operators when breaking a long line

[Eric Rahm]

Thanks everyone!

This got a bit derailed but it's clear that a unified style for binary
operators is desired.

I received several emails off-list in support of my original proposal, a
few people reiterated their preference for operators on the newline and we
rehashed that conversation. Overall given the back and forth it seems like
we're leaning towards going with unifying the style for splitting binary
operators by leaving them on the original line.

At this point I'll leave it to Ehsan, the code-style module owner, to make
the final decision.

-e

On Wed, Sep 6, 2017 at 12:30 PM, Eric Rahm <er...@mozilla.com> wrote:

> Hi folks-
>
> *Note: Previously we've discussed the placement of logical operators &&
> and ||; a decision was made and I do not wish to re-litigate that here*.
>
> Currently we have a somewhat convoluted set of rules about where to place
> boolean operators when breaking long lines [1]. Essentially we say that
> logical operators such as `&&` stay at the end, but boolean opertators
> such as `>` move to the new line. It's possible I'm misreading that
> (others certainly have [2]).
>
> This is incompatible with tools like clang-format [3] which support either
> leaving the operator at the end of the line or moving it to the new line,
> but not a half and half approach. It is very unlikely we could convince
> them to upstream such a change given we are the only organization that has
> requested it.
>
> I would like to propose that we adjust our style guide to remove the
> distinction between logical operators and boolean operators for the purpose
> of line breaking and use a unified style.
>
> *Examples*
>
> Current rules:
>
> if ((aArgument == 100) &&
> (anotherReallyLongArgument
>  > 2000)) {
>
> Proposed change if we unify the rules, favoring the logical operator
> placement:
>
> if ((aArgument == 100) &&
> (anotherReallyLongArgument >
>  2000)) {
>
> I want to emphasize this is a *pragmatic* change, our style guide is out
> of the norm, and the ability of our developers being able to use tools like
> clang-format is a big win.
>
> Please let me know what you think.
>
> -e
>
> [1] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_
> guide/Coding_Style#Operators
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1396515
> [3] https://bugzilla.mozilla.org/show_bug.cgi?id=1338105#c16
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Coding style: Placement of binary operators when breaking a long line

[Eric Rahm]

As I said, I was hoping to avoid rehashing this point, but the general
consensus from the last rather contentious post [1] was that changing from
the prevalent style of the codebase for primarily aesthetic reasons was
hard to justify (points about readability were made on both sides). Nick
pointed out that our code base very clearly tilts to the operators on the
end of the line style [2].

Carrying forward this wisdom for this case makes sense, but I'll admit
moving everything to the newline is certainly an option that would provide
a unified style which is my greater concern. I don't think we could
reasonably make that change until some sort of flag day when we convert
everything as the new style would clash with the prevalent style. I also
don't want to start a discussion about that again :)

-e

[1]
https://groups.google.com/d/msg/mozilla.dev.platform/LD_KCtmmH74/dLckkuFjBwAJ
[2]
https://groups.google.com/d/msg/mozilla.dev.platform/LD_KCtmmH74/ErPunKZ_BwAJ

On Wed, Sep 6, 2017 at 5:18 PM, Mike Hommey <m...@glandium.org> wrote:

> On Wed, Sep 06, 2017 at 12:30:58PM -0700, Eric Rahm wrote:
> > Hi folks-
> >
> > *Note: Previously we've discussed the placement of logical operators &&
> and
> > ||; a decision was made and I do not wish to re-litigate that here*.
> >
> > Currently we have a somewhat convoluted set of rules about where to place
> > boolean operators when breaking long lines [1]. Essentially we say that
> > logical operators such as `&&` stay at the end, but boolean opertators
> such
> > as `>` move to the new line. It's possible I'm misreading that (others
> > certainly have [2]).
> >
> > This is incompatible with tools like clang-format [3] which support
> either
> > leaving the operator at the end of the line or moving it to the new line,
> > but not a half and half approach. It is very unlikely we could convince
> > them to upstream such a change given we are the only organization that
> has
> > requested it.
> >
> > I would like to propose that we adjust our style guide to remove the
> > distinction between logical operators and boolean operators for the
> purpose
> > of line breaking and use a unified style.
> >
> > *Examples*
> >
> > Current rules:
> >
> > if ((aArgument == 100) &&
> > (anotherReallyLongArgument
> >  > 2000)) {
> >
> > Proposed change if we unify the rules, favoring the logical operator
> > placement:
> >
> > if ((aArgument == 100) &&
> > (anotherReallyLongArgument >
> >  2000)) {
>
> On a personal note, I find > 2000 as in the first sample more readable
> than the latter. So much so that I'd actually prefer the logical
> operators to be on the next line than boolean operator being on the
> previous.
>
> Mike
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Coding style: Placement of binary operators when breaking a long line

[Eric Rahm]

Hi folks-

*Note: Previously we've discussed the placement of logical operators && and
||; a decision was made and I do not wish to re-litigate that here*.

Currently we have a somewhat convoluted set of rules about where to place
boolean operators when breaking long lines [1]. Essentially we say that
logical operators such as `&&` stay at the end, but boolean opertators such
as `>` move to the new line. It's possible I'm misreading that (others
certainly have [2]).

This is incompatible with tools like clang-format [3] which support either
leaving the operator at the end of the line or moving it to the new line,
but not a half and half approach. It is very unlikely we could convince
them to upstream such a change given we are the only organization that has
requested it.

I would like to propose that we adjust our style guide to remove the
distinction between logical operators and boolean operators for the purpose
of line breaking and use a unified style.

*Examples*

Current rules:

if ((aArgument == 100) &&
(anotherReallyLongArgument
 > 2000)) {

Proposed change if we unify the rules, favoring the logical operator
placement:

if ((aArgument == 100) &&
(anotherReallyLongArgument >
 2000)) {

I want to emphasize this is a *pragmatic* change, our style guide is out of
the norm, and the ability of our developers being able to use tools like
clang-format is a big win.

Please let me know what you think.

-e

[1]
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Operators
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1396515
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1338105#c16
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Coding style: Argument alignment

[Eric Rahm]

Okay sounds like there's agreement on a).

Thanks everyone!
-e

On Tue, Aug 29, 2017 at 11:40 PM, Martin Thomson  wrote:

> On Wed, Aug 30, 2017 at 12:07 PM, L. David Baron 
> wrote:
> > I think I do this because (b) has the disadvantage that more code
> > changes require touching additional lines, which is both changes
> > blame and is extra work (although it's not extra work if we're using
> > clang-format tree-wide).  Option (b) is also more likely to lead to
> > overly long lines that require wrapping.
>
> NSS had a lot of option (b) and we agreed that it was bad for these
> reasons.  You also have to agree not to do this, another thing that
> NSS was infested with:
>
>   nsresult ShortFunction(witharg, and another);
>   void   Function2  (HasOnlyOne* arg);
>
> Does clang-format even *do* this?  AlignConsecutiveDeclarations is the
> closest I could find to a directive that would do this sort of crazy
> alignment, but that seems more likely to govern my example than
> argument lists.  For the record, we should not enable that either.
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Coding style: Argument alignment

[Eric Rahm]

Hi folks-

Do we explicitly state a preferred alignment of arguments in multi-line
function declarations (primarily in the context of C++) [1]? This question
has come up in regards to using clang-format for cleaning up code [2] and
it would be helpful to be able to reference a concrete example in the style
guide.

Given this example:

int
Foo(const nsACstring& aStr,
mozilla::UniquePtr&& aBuffer,
nsISupports* aOptionalThing = nullptr);

Should the reformatted style:

a) *stay the same*
b) align the argument names like so:

int
Foo(const nsACstring& aStr,
mozilla::UniquePtr&& aBuffer,
nsISupports*  aOptionalThing = nullptr);

c) some other option

-e

[1]
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1378250
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: Use nsStringFwd.h for forward declaring strings

[Eric Rahm]

Hi all-

In bug 1391803  I'm
removing all manually forward declared string types and replacing them with
a #include "nsStringFwd.h". Moving forward please use that include file if
you want to forward declare a string class.

In the next few weeks we plan on changing the underlying class names and
statements like 'class nsCString;' will no longer work.

-e
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: New string types: nsAutoStringN<> and nsAutoCStringN<>

[Eric Rahm]

On Mon, Aug 21, 2017 at 8:32 AM, Jonathan Kew  wrote:

>
> Wouldn't it be more "modern Gecko-ish", though, to drop the "ns" prefix,
> and perhaps put Auto[C]String into the mozilla namespace?
>
>
As Nick said, renaming all the things is a job for another day :)

Coincidentally I have some pending changes that affect the internal naming
of all of our strings. Externally (outside of xpcom/string) there will be
no discernible change but I *could* move everything to the mozilla
namespace and drop the 'ns' prefix. We could then gradually migrate to the
new naming scheme externally. I think we'd eventually want to move the
include path to 'mozilla/String.h' as well if we went this route, in the
interim we could just have stubs that redirect to the mozilla path.

I'm not sure how much backing that has -- we'd be going from nsString =>
String which is pretty close to std::string -- it would be interesting to
get some feedback.

-e
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Is Pico Synth Synthesis used anymore?

[Eric Rahm]

In bug 1389598 [1] I'm removing a few remaining gonk (b2g) references. We
have support for "pico speech synthesis" that implies it's gonk-only [2].
Does anyone know if we're using it elsewhere? If so I'll add a note in the
configure script, if not I'll file a bug for removal.

-e

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1389598
[2]
http://searchfox.org/mozilla-central/rev/6482c8a5fa5c7446e82ef187d1a1faff49e3379e/old-configure.in#4027-4037
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Actually-Infallible Fallible Allocations

[Eric Rahm]

Both AppendElements and SetLength default initialize the values, there's no
intermediate uninitialzed state. SetCapacity doesn't initialize the values,
but they're also not indexable -- we have release bounds assertions --
unless you try really hard.

nsTArray doesn't support emplace although it does have AppendElement(T&&),
but that wouldn't really help in this case. It's possible we could add that
of course!

-e

On Tue, Aug 1, 2017 at 1:11 PM, Kris Maglione <kmagli...@mozilla.com> wrote:

> On Tue, Aug 01, 2017 at 12:57:31PM -0700, Eric Rahm wrote:
>
>> nsTArray has various Append methods, in this case just using the
>> infallible
>> AppendElements w/o out a SetCapacity call would do the job. Another option
>> would be to use SetLength which would default initialize the new elements.
>>
>> If we're trying to make things fast-but-safeish in this case, the
>> preferred
>> way is probably:
>>
>> auto itr = AppendElements(length, fallible);
>> if (!itr) ...
>>
>> // no bounds checking
>> for (...; i++, itr++)
>>   auto& mSocket = *itr;
>>
>> // bounds checking
>> for (...)
>>auto& mSocket = *sockets[i];
>>
>> In general I agree the pattern of fallibly allocating and then fallibly
>> appending w/o checking the return value is a bit silly. Perhaps we should
>> just mark the fallible version MUST_USE? It looks like it's commented out
>> for some reason (probably a ton of bustage).
>>
>
> Honestly, I think the Vector infallible* methods are a much cleaner way to
> handle this, especially when we need something like infallibleEmplaceBack.
> They tend to encourage writing more efficient code, with fewer
> reallocations and allocation checks. And I think the resulting code tends
> to be safer than AppendElements followed by unchecked raw pointer access,
> placement `new`s, and an intermediate state where we have a bunch of
> indexable but uninitialized elements in the array.
>
> That's been my experience when reading and writing code using the various
> approaches, anyway.
>
>
> On Tue, Aug 1, 2017 at 12:18 PM, Kris Maglione <kmagli...@mozilla.com>
>> wrote:
>>
>> On Tue, Aug 01, 2017 at 12:31:24PM -0400, Alexis Beingessner wrote:
>>>
>>> I was recently searching through our codebase to look at all the ways we
>>>> use fallible allocations, and was startled when I came across several
>>>> lines
>>>> that looked like this:
>>>>
>>>> dom::SocketElement  = *sockets.AppendElement(fallible);
>>>> ...
>>>> So in isolation this code is saying "I want to handle allocation
>>>> failure"
>>>> and then immediately not doing that and just dereferencing the result.
>>>> This
>>>> turns allocation failure into Undefined Behaviour, rather than a process
>>>> abort.
>>>>
>>>> Thankfully, all the cases where I found this were preceded by something
>>>> like the following:
>>>>
>>>> uint32_t length = socketData->mData.Length();if
>>>> (!sockets.SetCapacity(length, fallible)) {   JS_ReportOutOfMemory(cx);
>>>>  return NS_ERROR_OUT_OF_MEMORY;}for (uint32_t i = 0; i <
>>>> socketData->mData.Length(); i++) {dom::SocketElement  =
>>>> *sockets.AppendElement(fallible);
>>>>
>>>>
>>>>
>>>> So really, the fallible AppendElement *is* infallible, but we're just
>>>> saying "fallible" anyway. I find this pattern concerning for two
>>>> reasons:
>>>>
>>>>
>>> The MFBT Vector class handles this with a set of infallibleAppend[1]
>>> methods that assert that space has been reserved for the new elements
>>> before they're called. If we're using this pattern elsewhere without the
>>> same safeties, either those places should probably switch to using
>>> Vectors,
>>> or we should probably add similar checked infallible methods to nsTArray.
>>>
>>>
>>> [1]: http://searchfox.org/mozilla-central/rev/bbc1c59e460a27b2092
>>> 9b56489e2e55438de81fa/mfbt/Vector.h#707-733
>>>
>>> ___
>>> dev-platform mailing list
>>> dev-platform@lists.mozilla.org
>>> https://lists.mozilla.org/listinfo/dev-platform
>>>
>>>
> --
> Kris Maglione
> Senior Firefox Add-ons Engineer
> Mozilla Corporation
>
> Science is interesting, and if you don't agree you can fuck off.
> --Nigel Calder, former editor of New Scientist
>
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Actually-Infallible Fallible Allocations

[Eric Rahm]

nsTArray has various Append methods, in this case just using the infallible
AppendElements w/o out a SetCapacity call would do the job. Another option
would be to use SetLength which would default initialize the new elements.

If we're trying to make things fast-but-safeish in this case, the preferred
way is probably:

auto itr = AppendElements(length, fallible);
if (!itr) ...

// no bounds checking
for (...; i++, itr++)
   auto& mSocket = *itr;

// bounds checking
for (...)
auto& mSocket = *sockets[i];

In general I agree the pattern of fallibly allocating and then fallibly
appending w/o checking the return value is a bit silly. Perhaps we should
just mark the fallible version MUST_USE? It looks like it's commented out
for some reason (probably a ton of bustage).

-e

On Tue, Aug 1, 2017 at 12:18 PM, Kris Maglione 
wrote:

> On Tue, Aug 01, 2017 at 12:31:24PM -0400, Alexis Beingessner wrote:
>
>> I was recently searching through our codebase to look at all the ways we
>> use fallible allocations, and was startled when I came across several
>> lines
>> that looked like this:
>>
>> dom::SocketElement  = *sockets.AppendElement(fallible);
>> ...
>> So in isolation this code is saying "I want to handle allocation failure"
>> and then immediately not doing that and just dereferencing the result.
>> This
>> turns allocation failure into Undefined Behaviour, rather than a process
>> abort.
>>
>> Thankfully, all the cases where I found this were preceded by something
>> like the following:
>>
>> uint32_t length = socketData->mData.Length();if
>> (!sockets.SetCapacity(length, fallible)) {   JS_ReportOutOfMemory(cx);
>>  return NS_ERROR_OUT_OF_MEMORY;}for (uint32_t i = 0; i <
>> socketData->mData.Length(); i++) {dom::SocketElement  =
>> *sockets.AppendElement(fallible);
>>
>>
>>
>> So really, the fallible AppendElement *is* infallible, but we're just
>> saying "fallible" anyway. I find this pattern concerning for two reasons:
>>
>
> The MFBT Vector class handles this with a set of infallibleAppend[1]
> methods that assert that space has been reserved for the new elements
> before they're called. If we're using this pattern elsewhere without the
> same safeties, either those places should probably switch to using Vectors,
> or we should probably add similar checked infallible methods to nsTArray.
>
>
> [1]: http://searchfox.org/mozilla-central/rev/bbc1c59e460a27b2092
> 9b56489e2e55438de81fa/mfbt/Vector.h#707-733
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: More Rust code

[Eric Rahm]

Interesting points.

   - *using breakpad* - was the problem that creating wrappers to access
   the c/c++ code was too tedious? Could bindgen help with that, if not it
   would be interesting gather some details about why it wouldn't work and
   file bugs against it.
   - *pingsender* - was something like https://hyper.rs/ not around when
   you were working on it or is this a case of finding the things you want can
   be difficult in rust-land? Either way it might be a good idea for us to put
   together a list of "sanctioned" crates for various tasks.

-e

On Wed, Jul 12, 2017 at 1:36 AM, Gabriele Svelto 
wrote:

> On 10/07/2017 12:29, Nicholas Nethercote wrote:
> > Hi,
> > What are the obstacles? Here are some that I've heard.
> > [...]
> > Anything else?
>
> In the past year I wrote two tools (minidump-analyzer and pingsender)
> that ship with Firefox but are separate executables so both were good
> candidates for being written in Rust in the first place. I implemented
> both in C++ because of different issues:
>
> - The minidump-analyzer relies on Google Breakpad machinery that is
> currently C++. It's being rewritten in Rust but it's not there yet and
> since we needed it to quickly get more crash information going the Rust
> route would have simply taken too long. It is a good candidate for being
> rewritten in Rust in the coming 12 months though.
>
> - The pingsender talks to our telemetry infrastructure so needs
> HTTP/HTTPS functionality but without linking with libxul. In this case
> if I had a crate that offered HTTP functionality I would have gone
> straight to Rust but alas it wasn't there. So instead it's a blob of
> platform-specific C++ that dlopen()s libcurl on Mac and Linux and relies
> on WININET on Windows.
>
>  Gabriele
>
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Profiling nightlies on Mac - what tools are used?

[Eric Rahm]

DMD [1], although it's a bit busted on mac right now [2] I'd prefer if it
didn't get more busted :)

-e

[1] https://developer.mozilla.org/en-US/docs/Mozilla/Performance/DMD
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1371397

On Mon, Jun 19, 2017 at 3:03 PM, Chris Cooper  wrote:

> Hey all,
>
> The build peers are looking to change the way that nightlies are created
> on Mac as we switch to cross-compilation. Specifically, we're looking at
> stripping the nightlies to avoid an as-of-yet undiagnosed performance
> discrepancy vs native builds[1], but also to make the nightly configuration
> match what we ship on beta/release (stripped).
>
> Of course, stripping removes the symbols, and while we believe we have a
> solution for re-acquiring symbols that works for the Gecko Profiler, we
> realize
> that people out there may be using other profiling tools.
>
> If you profile on Mac, now is your chance to speak up. What other
> profiling tools do you use that we should be aware of?
>
> cheers,
> --
> coop
>
> 1. https://bugzilla.mozilla.org/show_bug.cgi?id=1338651
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Improving visibility of compiler warnings

[Eric Rahm]

I think we disable it for local builds because we don't control what
versions of tools folks use. So clang vFOO might spit out errors we don't
see in clang vBAR and it would be a huge pain if those failed locally even
though they'd be fine in automation.

-e

On Thu, May 25, 2017 at 4:07 PM, Andrew McCreight 
wrote:

> On Thu, May 25, 2017 at 4:03 PM,  wrote:
>
> > On Friday, May 26, 2017 at 6:03:02 AM UTC+12, Chris Peterson wrote:
> > > On 2017-05-25 5:31 AM, Ehsan Akhgari wrote:
> > > > On 05/19/2017 02:44 PM, Gregory Szorc wrote:
> > > >> `mach build` attempts to parse compiler warnings to a persisted
> > > >> "database."
> > > >> You can view a list of compiler warnings post build by running `mach
> > > >> warnings-list`. The intent behind this feature was to make it easier
> > to
> > > >> find and fix compiler warnings. After all, something out of sight is
> > > >> out of
> > > >> mind.
> > >
> > > Given that we treat warnings as errors on CI and most directories that
> > > opt out of warnings-as-errors are third-party code
> > > (ALLOW_COMPILER_WARNINGS in moz.build [1]), I don't think we need to
> > > make the warning list any more visible. A warning regression in nearly
> > > all first-party code is already treated as an error.
> > >
> > >
> > > [1]
> > > https://searchfox.org/mozilla-central/search?q=ALLOW_
> > COMPILER_WARNINGS=true=moz.build
> >
> > Tangentially: "we treat warnings as errors on CI" -- It'd be great if
> > local builds could have that behavior by default, so we (or just I?)
> don't
> > get caught by surprise when warnings-as-errors appear after landing.
> >
> > Or at least: Can I opt-in locally? (I tried 'ac_add_options
> > --enable-warnings-ar-errors' in mozconfig, but that was not recognized.)
> >
>
> This has worked for me before:
> ac_add_options --enable-warnings-as-errors
>
>
> ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> >
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Scope of XML parser rewrite?

[Eric Rahm]

Thanks Henri, I think we can find a middle ground so as to avoid a ton of
scope creep but leave the door open to a better iterative solution. See
notes inline.

-e

On Wed, May 24, 2017 at 11:18 PM, Henri Sivonen 
wrote:

> On Wed, May 24, 2017 at 10:11 AM, Henri Sivonen 
> wrote:
> >> Our current interface is UTF-16, so that's my target for now. I think
> >> whatever cache-friendliness would be lost converting from UTF-16 ->
> UTF-8 ->
> >> UTF-16.
> >
> > I hope this can be reconsidered, because the assumption that it would
> > have to be UTF-16 -> UTF-8 -> UTF-16 isn't accurate.
>
> I see that this part didn't get an on-list reply but got an blog reply:
> http://www.erahm.org/2017/05/24/a-rust-based-xml-parser-for-firefox/
>

Yes sorry, I should have followed up here as well!


> I continue to think it's a bad idea to write another parser that uses
> UTF-16 internally. Even though I can see your desire to keep the
> project tightly scoped, I think it's fair to ask you to expand the
> scope a bit by 1) adding a way to pass Latin-1 data to text nodes
> directly (and use this when the the parser sees a text node is all
> ASCII) and 2) replacing nsScanner with a bit of new buffering code
> that takes bytes from the network and converts them to UTF-8 using
> encoding_rs.
>
> We've both had the displeasure of modifying nsScanner as part of a
> security fix. nsScanner isn't valuable code that we should try to
> keep. It's no longer scanning for anything. It's just an
> over-complicated way of maintaining a buffer of UTF-16 data. While
> nsScanner and the associated classes are a lot of code, they do
> something simple that should be done in quite a bit less code, so as
> scope creep, replacing nsScanner should be a drop in a bucket
> effort-wise compared to replacing expat.
>
> I think it's super-sad if we get another UTF-16-using parser because
> replacing nsScanner was scoped out of the project.
>

Limiting to modifying nsScanner might be an option, but probably not
changing all callers that use the nsAString interface. I guess we can just
UTF-16 => UTF-8 those and file a bunch of follow ups?

One thing we've ignored are all the consumers expect output to be UTF-16,
so there's a fair amount of work on that side as well.

Maybe a reasonable approach is to use a UTF-8 interface for the replacement
Rust library and work on a staged rollout:

   1. Start just converting UTF-16 => UTF-8 for input at the nsExpatDriver
   level, UTF-8 => UTF-16 for output
   2. Modify/replace nsScanner with something that works with UTF-8 (and
   encoding_rs?), convert UTF-16 => UTF-8 for the nsAString methods
   3. Follow up replacing nsAString methods with UTF-8 versions
   4.  Look into whether modifying the consumers of the tokenized data to
   handle UTF-8 is reasonable, follow up as necessary

WDYT?


> --
> Henri Sivonen
> hsivo...@hsivonen.fi
> https://hsivonen.fi/
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Start logging at runtime (Firefox 52)

[Eric Rahm]

You're going to have a pretty bad day if you route the rather verbose http
logs through the browser console. In general I think it's feasible, we
could probably add a pref for it and just punt messages to the console
service instead of fprintf_stderr.

We have a bug filed for working around the root cause,
https://bugzilla.mozilla.org/show_bug.cgi?id=1345046 for a more robust
solution, but that doesn't currently have an owner.


On Tue, May 23, 2017 at 4:32 PM, Chris Pearce <cpea...@mozilla.com> wrote:

> On Wednesday, May 24, 2017 at 2:22:49 AM UTC+12, Valentin Gosu wrote:
> > I think the you can get around the issue by setting the
> > security.sandbox.content.level pref to 0 while you're debugging.
> >
> > On 23 May 2017 at 08:32, Shih-Chiang Chien <sch...@mozilla.com> wrote:
> >
> > > Which platform you're using? For windows it seems to be solved by
> > > https://bugzilla.mozilla.org/show_bug.cgi?id=1320458, however other
> > > platforms are not fixed yet.
> > >
> > >
> > > Best Regards,
> > > Shih-Chiang Chien
> > > Mozilla Taiwan
> > >
> > > On Tue, May 23, 2017 at 11:59 AM, Chris Pearce <cpea...@mozilla.com>
> > > wrote:
> > >
> > > > On Sunday, November 27, 2016 at 5:59:27 AM UTC+13, Valentin Gosu
> wrote:
> > > > > Hi everyone,
> > > > >
> > > > > (I meant to send this mail a few weeks ago but forgot it in my
> Drafts
> > > > > folder.)
> > > > >
> > > > > With the landing of Bug 1303762 (Firefox 52), we now have a way for
> > > users
> > > > > to enable logging without restarting the browser, and without
> having to
> > > > > know what an environment variable is.
> > > > >
> > > > > We've added a new Logging section to about:networking. When a user
> > > > > encounters a bug, all they have to do is open that page, click on
> > > "Start
> > > > > Logging", reproduce the bug, then click on "Stop Logging" and
> upload
> > > the
> > > > > logs. The buttons will be disabled if the MOZ_LOG_FILE or MOZ_LOG
> env
> > > > > variables have been defined.
> > > > > The log modules are automatically set to the most common networking
> > > > > modules, but you may instruct the bug reporters to change them -
> just
> > > > tell
> > > > > them the string.
> > > > >
> > > > > This is very useful for bugs that are harder to reproduce once you
> > > > restart
> > > > > the browser.
> > > > >
> > > > > There are a bunch of improvements that we could make to the UI, so
> > > please
> > > > > feel free to send me your feedback and patches. Many thanks to
> Honza
> > > > > Bambas, Eric Rahm, Jared Wein, Patrick McManus and all others that
> > > helped
> > > > > with the implementation and review of this bug and its
> dependencies.
> > > > >
> > > > > https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/
> > > > HTTP_logging?document_saved=true#Using_aboutnetworking
> > > >
> > > > This seems super handy, but I tried it out and it seems to only
> affect
> > > the
> > > > parent process, and not the sub-processes of Firefox?
> > > >
> > > > ___
> > > > dev-platform mailing list
> > > > dev-platform@lists.mozilla.org
> > > > https://lists.mozilla.org/listinfo/dev-platform
> > > >
> > > ___
> > > dev-platform mailing list
> > > dev-platform@lists.mozilla.org
> > > https://lists.mozilla.org/listinfo/dev-platform
> > >
>
> I'm using Firefox 54b10 32bit on Windows 10 x64.
>
> Changing the sandbox pref worked, though it required a restart.
>
> Has anyone thought of routing MOZ_LOGs to the Browser Console? How hard
> would that be?
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Scope of XML parser rewrite?

[Eric Rahm]

I was hoping to write a more thorough blog post about this proposal (I have
some notes in a gist [1]), but for now I've added comments inline. The main
takeaway here is that I want to do a bare-bones replacement of just the
parts of expat we currently use. It needs to support DTD entities, have a
streaming interface, and support XML 1 v4. That's it, no new features, no
rewrite of our entire XML stack.

-e

[1] https://gist.github.com/EricRahm/f718c4d8a862cc08b69d7d4290c02927

On Mon, May 22, 2017 at 11:43 PM, Henri Sivonen 
wrote:

> In reference to: https://twitter.com/nnethercote/status/866792097101238272
>
> Is the rewrite meant to replace expat only or also some of our old
> code on both above and below expat?
>

Just expat.


> Back in 2011, I wrote a plan for rewriting the code around expat
> without rewriting expat itself:
> https://wiki.mozilla.org/Platform/XML_Rewrite
> I've had higher-priority stuff to do ever since...
>
>
Yes, I've seen this. It explicitly calls out not replacing expat, so the
plans are mostly orthogonal.


> (The above plan talks about pushing UTF-16 to the XML parser and
> having deep C++ namespaces. Any project starting this year should make
> the new parser use UTF-8 internally for cache-friendliness and use
> less deep C++ namespaces.)
>

Our current interface is UTF-16, so that's my target for now. I think
whatever cache-friendliness would be lost converting from UTF-16 -> UTF-8
-> UTF-16.


> Also, I think the decision of which XML version to support should be a
> deliberate decision and not an accident. I think the reasonable
> choices are XML 1.0 4th edition (not rocking the boat) and reviving
> XML5 (original discussion: https://annevankesteren.nl/2007/10/xml5 ,
> latest draft: https://ygg01.github.io/xml5_draft/). XML 1.1 is dead.
> XML 1.0 5th edition tried to have the XML 1.0 cake and eat the XML 1.1
> cake too and expanded the set of documents that parser doesn't reject.
> Any of the newly well-forming documents would be incompatible with 4th
> ed. and earlier parsers, which would be a break from universal XML
> interop. I think it doesn't make sense to relax XML only a bit. If XML
> is to be relaxed (breaking interop in the sense of starting to accept
> docs that old browsers would show the Yellow Screen of Death on), we
> should go all the way (i.e. XML5).
>
>
My current goal is a drop-in replacement for expat with just the features
gecko cares about, so just 1.0 version 4 I guess. It's possible whatever we
end up with could be merged with another library when XML5 is settled, but
I don't want to wait for that.


> Notably, it looks like Servo already has an XML5 parser written in Rust:
> https://github.com/servo/html5ever/tree/master/xml5ever
>
>
Yes, this lacks DTD support (and 1.0 support).


> The tweets weren't clear about whether xml5ever had been considered,
> but https://twitter.com/eroc/status/866808814959378434 looks like it's
> talking about writing a new one.
>
>
Correct, I looked at xml5ever and spoke with some folks on #servo about it.
It doesn't meet Firefox's requirements.


> It seems like integrating xml5ever (as opposed to another XML parser
> written in Rust) into Gecko would give some insight into how big a
> deal it would be to replace Gecko's HTML parser with html5ever
> (although due to document.write(), HTML is always a bigger deal
> integration-wise than XML).
>
>
That's a non-goal for me, but I can see how it would be useful.


> (If the outcome here is to do XML5, we should make sure the spec is
> polished enough at the WHATWG in order not to a unilateral thing in
> relative secret.)
>
>
That is not my current goal, but that seems reasonable regardless of this
project.


> --
> Henri Sivonen
> hsivo...@hsivonen.fi
> https://hsivonen.fi/
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Is it OK to make allocations that intentionally aren't freed? (was: Re: Is Big5 form submission fast enough?)

[Eric Rahm]

I second Kris' concern re: memory, particularly given this is in multiple
processes. I'd suggest something more along the lines of a timeout; AFAICT
'memory-pressure' isn't actually fired in low-memory situations (it's still
useful, and registering for it and handling it would make sense for
anything that's caching large amounts of data). I'd certainly want to see
measurements on what the actual overhead is.

In general I'm not super concerned about leaking at shutdown, but of course
as you noted V, lsan, etc are and it makes life harder for our leak
checking frameworks. Kris and Jeff's suggestions seem reasonable.

I'd be less concerned about overhead if we had a good way of sharing these
static tables across processes (ICU seems like a good candidate as well).

-e

On Fri, May 19, 2017 at 11:58 AM, Kris Maglione 
wrote:

> On Fri, May 19, 2017 at 08:44:58AM +0300, Henri Sivonen wrote:
>
>> The downsides would be that the memory for the tables wouldn't be
>> reclaimed if the tables aren't needed anymore (the browser can't
>> predict the future) and executions where any of the tables has been
>> created wouldn't be valgrind-clean.
>>
>
> If we do this, it would be nice to flush the tables when we get a
> memory-pressure event, which should at least mitigate some of the effects
> for users on memory-constrained systems.
>
> And is there a reason ClearOnShutdown couldn't be used to deal with
> valgrind issues?
>
> That said, can we try to get some telemetry on how often we'd need to
> build these tables, and how likely they are to be needed again in the same
> process, before we make a decision?
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Removing Jemalloc 4

[Eric Rahm]

Having been involved with jemalloc 3/4/5 work as well, I agree with Mike's
conclusions.

-e

On Mon, May 15, 2017 at 6:19 PM, Nicholas Nethercote  wrote:

> Just to add some context: glandium is deeply familiar with jemalloc4's
> internals, having submitted numerous patches and fixes to it. And he has
> spent *significant* time and effort on multiple occasions, on multiple
> versions of jemalloc4, trying to avoid the performance regressions, without
> success. If he can't get it into a state suitable for our use, I have
> trouble imagining who else could.
>
> In other words, this is not a hasty or premature decision, but one made
> reluctantly based on experience.
>
> Nick
>
> On Tue, May 16, 2017 at 10:14 AM, Mike Hommey  wrote:
>
> > Hi,
> >
> > We've tried to get off mozjemalloc for, apparently, close to 5 years
> > (date of the filing of bug 762449). We've had memory usage regressions
> > (like bug 1219914), and we've had perf regressions as per talos numbers
> > (things like bug 1138999), and those have never gone away (with
> > variations with each update of jemalloc >= 3).
> >
> > My best bet at this point is that it's "just" a consequence of the
> > difference in memory layout due to the differences in how the allocators
> > work. That doesn't make it more okay.
> >
> > Many updates to recent versions of jemalloc have been painful (usually
> > breaking everything except linux), and the current tip of the jemalloc
> > dev branch is not making things any better (see bug 1357301).
> >
> > Furthermore, bug 1361258 has started to modify mozjemalloc with new
> > features for stylo, further deepening the API surface between both
> > allocators. While what was added in bug 1361258 is possible to implement
> > for jemalloc 4, the burden of continuing to maintain both allocators is
> > not really appealing considering the perspective of ever switching.
> >
> > As much as it pains me, it's time to admit that switching to jemalloc >=
> > 3 is not going to happen, and pull the plug once and for all. This
> > decision has taken way too long to be made, and I apologize for that.
> >
> > This will happen with the landing of bug 1363992 in a few hours.
> >
> > As for the things we were hoping jemalloc >= 3 would allow us to do
> > (essentially, heap partitioning), we'll be working on getting that on
> > mozjemalloc shortly (bug 1361258 and followups will actually get us
> > close on the necessary infrastructure), as well as cleaning up its code
> > (I have a local patch queue that removes 15% of jemalloc.c), and
> > probably converting it to C++ (at least for some RAII benefits, and
> > converting some of the gory macros to templates)
> >
> > Mike.
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> >
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Have you run 'mach bootstrap' lately?

[Eric Rahm]

Didn't it somehow cause builds to fail? A gentle reminder is probably fine.
TBH I'd be fine if it auto-updated but maybe I'm in the minority.

On Fri, May 12, 2017 at 11:34 AM, Ted Mielczarek  wrote:

> On Fri, May 12, 2017, at 10:45 AM, Sylvestre Ledru wrote:
> > Would it be possible to add a check like:
> > "You haven't updated your local configuration since XX days, please
> > consider running
> > mach bootstrap ?"
>
> We've had mach produce nag messages like that in the past and they have
> been universally disliked, FWIW.
>
> -Ted
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Preferences::RegisterCallback and variants will now do exact, not prefix, matches

[Eric Rahm]

This doesn't affect the behavior of |Preferences::AddStrongObserver| which
does prefix matching, correct?

-e

On Tue, Mar 21, 2017 at 12:07 PM, Boris Zbarsky  wrote:

> I have just landed a change[1] which changes 
> Preferences::RegisterCallback/RegisterCallbackAndCall
> to do an exact, not prefix, match on the passed-in string.
>
> So if you do:
>
>   Preferences::RegisterCallback(MyFunc, "foo");
>
> and the preference named "foobar" changes, MyFunc will no longer be
> called.  Most consumers did not expect this behavior, and a number were
> buggy as a result.
>
> The old behavior is still available via a new
> RegisterPrefixCallback/RegisterPrefixCallbackAndCall/UnregisterPrefixCallback
> API.  These can be used to observe all preferences whose names start with
> the given string.
>
> One caveat: It's very important to correctly pair up the registration and
> unregistration functions.  If you RegisterPrefixCallback but then
> UnregisterCallback, it will not actually unregister the callback.
>
> -Boris
>
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1348331
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: windows build anti-virus exclusion list?

[Eric Rahm]

It looks like there's bug 1188823 [1] for enabling fastlink to improve link
times, but that's in a bad way right now.

FWIW on my ThankPad laptop I have clobber build times in the 35 - 40 minute
range, so even things that seem small for a 16 core desktop are a bigger
win for me.

-e

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1188823

On Fri, Mar 17, 2017 at 11:20 AM, Ben Kelly  wrote:

> On Fri, Mar 17, 2017 at 1:36 PM, Ted Mielczarek 
> wrote:
>
> > Back to the original topic, I recently set up a fresh Windows machine
> > and I followed the same basic steps (enable performance power mode,
> > whitelist a bunch of stuff in Windows Defender) and my build seemed
> > basically CPU-bound[1] during the compile tier. Disabling realtime
> > protection in Defender made it *slightly* better[2] but didn't have a
> > large impact on the overall build time (something like 20s out of ~14m
> > total for a clobber).
> >
>
> The 14min measurement must have been for a partial build.  With defender
> disabled the best I can get is 18min.  This is on one of the new lenovo
> p710 machines with 16 xeon cores.
>
> I definitely observed periods where it was not CPU bound.  For example, at
> the end of the js lib build I observed a single cl.exe process sit for ~2
> minutes while no other work was being done.  I also saw link.exe take a
> long time without parallelism, but i think that's a known issue.
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: windows build anti-virus exclusion list?

[Eric Rahm]

I filed meta bug 1326328 [1] a few months ago for tracking things we can do
to improve Windows build times. It would be great to file / block existing
bugs against the meta bug to track these issues. There hasn't been much
traction on any of the bugs though, perhaps a good topic for the next
all-hands.

A quick summary of what we have:

   - Bug 1095293 [2] - Use msvc's '/MP' option
   - Bug 1321922 [3] - Use symlinks on windows to avoid copying files into
   the objdir
   - Bug 1326329 [4] - Each time we call 'cl' we spawn 5 processes
   - Bug 1326333 [5] - Build from legit msvc project files to leverage
   msbuild's multiprocess improvements
   - Bug 1326353 [6] - Reduce the amount of console output when building

Integrating suggestions for first-time builders into `mach bootstrap` would
certainly be great (even if it's just a link to a "10 crazy things that
will make your windows build faster!" article). I didn't see a bug for
programmatically disabling power save mode while building, but I recall
that being a potential option.

-e

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1326328
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1095293
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1321922
[4] https://bugzilla.mozilla.org/show_bug.cgi?id=1326329
[5] https://bugzilla.mozilla.org/show_bug.cgi?id=1326333
[6] https://bugzilla.mozilla.org/show_bug.cgi?id=1326353

On Fri, Mar 17, 2017 at 10:36 AM, Ted Mielczarek  wrote:

> On Fri, Mar 17, 2017, at 01:12 PM, Chris Peterson wrote:
> > On 3/17/2017 1:45 AM, Honza Bambas wrote:
> > > I have a very similar setup, with even way more exceptions added, but
> > > none of them has the desired effect. Unfortunately, the only way to
> make
> > > MsMpEng shut up is to disable run-time protection completely for the
> > > time of the build. I think it's a bug in Defender.
> >
> > Maybe `mach build` can temporarily disable Defender when building?
>
> You can't programmatically control Windows Defender, even as an
> Administrator. This is a security precaution from Microsoft. It's
> configured with a special user account. I looked into this recently
> because I thought it would be nice if *something* in the build system or
> bootstrap could at least let you know if your build directories were not
> in the list of exclusions.
>
> Back to the original topic, I recently set up a fresh Windows machine
> and I followed the same basic steps (enable performance power mode,
> whitelist a bunch of stuff in Windows Defender) and my build seemed
> basically CPU-bound[1] during the compile tier. Disabling realtime
> protection in Defender made it *slightly* better[2] but didn't have a
> large impact on the overall build time (something like 20s out of ~14m
> total for a clobber).
>
> Ideally we should have this stuff as part of `mach bootstrap` or similar
> so everyone gets their machine configured properly for the fastest
> builds possible.
>
> Related, my next steps were that I was planning to figure out how to
> gather an xperf profile of the entire build process to see if there were
> any obvious speedups left from a system perspective (the resource usage
> graph shows the obvious inefficiencies left that are already known:
> configure + the non-compile tiers), but UIforETW hung when I tried to
> use it to do so and I haven't followed up yet.
>
> -Ted
>
> 1. http://people.mozilla.org/~tmielczarek/build-resource-usage.svg
> 2.
> https://people-mozilla.org/~tmielczarek/build-resource-
> usage-no-defender.svg
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Heads up: archive.m.o is no longer updating tinderbox builds, AWSY is effectively dead

[Eric Rahm]

Can you add these details to the bug? We should probably take the
conversation on the best way to fix bustage there.

Given the fallout (read: memory regression tracking is gone) and, as you
noted, we have the ability to continue posting taskcluster builds to
archive.m.o, we should at least continue to post builds to archive.m.o for
at least grace period in order to give AWSY, mozdownload, and others time
to implement a switch to a purely taskcluster-only solution.

-e

On Tue, Mar 7, 2017 at 7:43 AM,  wrote:

> On Monday, March 6, 2017 at 6:08:09 PM UTC-5, Boris Zbarsky wrote:
> > On 3/6/17 5:29 PM, Mike Hommey wrote:
> > > You can get the builds through the taskcluster index.
> >
> > Does that have the same lifetime guarantees as archive.mozilla.org?
> >
> > -Boris
>
> So, to be clear..
>
> This thread is talking about http://archive.mozilla.org/
> pub/firefox/tinderbox-builds/**
>
> Oldest archive files in there (for autoland) is May 26'th 2016.
>
> On taskcluster's index, there are many ways to get the data (not just by
> buildid) there is date, there is .latest. and there is by revision, but to
> use a date based solution to help answer this, oldest on taskcluster is
> also May 26...
>
> https://tools.taskcluster.net/index/artifacts/#gecko.v2.
> autoland.pushdate.2016.05.26/gecko.v2.autoland.pushdate.2016.05.26
>
> Releng has traditionally considered the archive.m.o location for
> on-checkin builds to be an implementation detail and not suitable outside
> of our automation.  We've also published things to both archive.m.o and
> taskcluster index for a long time now (we even publish osx and windows jobs
> there, so its not like you have to keep both solutions implemented).
>
> Had we realized anyone was using the `tinderbox-builds/` location for
> things before this, I feel we would have notified those projects and the
> community in general more widely when we were to shut them off.
>
> I apologize this broke anyone, but there is a clear path forward. We
> explicitly preserved the archive.m.o uploads for taskcluster nightlies,
> into http://archive.mozilla.org/pub/firefox/nightly/
>
> ~Justin Wood (Callek)
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: All the processes

[Eric Rahm]

On Mon, Mar 6, 2017 at 4:27 PM,  wrote:

> Just about the "4 good, 8 bad" part, it seems quite arbitrary -- Wouldn't
> that be hardware-dependent?
> I would think users with "only" 1GB may have different needs and
> expectations from users with 16+GB.
>

It's more about acceptable memory usage in comparison to other browsers.
FWIW a user with 1GB might have a better experience with multiple processes
due to less crashes from virtual address space fragmentation. It depends on
what we consider our user's needs: less crashes or less swapping?


> Intuitively I don't grasp how each content process can add that much more
> memory that it would become a "major problem" jumping from 4 to 8 -- Are
> these measurements accessible somewhere, to get a sense of the magnitudes
> involved?
>
>
I'd suggest looking at the memory overhead of Chrome's individual processes
as compared to ours, it's pretty impressive. My blog posts on our own e10s
memory usage [1] and comparison to other browsers [2] have further details.
I'm planning on performing the same measurements again to see how we do a
year later with e10s-multi enabled (along with the GPU process, etc).


> After that, of course for each machine there may be a limit we would want
> to enforce, so this discussion here is still needed.
>

Sure, this is a harder number to nail down.

-e

[1]
http://www.erahm.org/2016/02/11/memory-usage-of-firefox-with-e10s-enabled/
[2] http://www.erahm.org/2016/02/12/are-they-slim-yet/

Thanks,
> Gerald
>
> On Tuesday, March 7, 2017 at 9:13:26 AM UTC+11, Nicholas Nethercote wrote:
> ...
> > Now for the reason I raised this: the major downside of using multiple
> > processes is that it increases memory usage. Recent-ish measurements
> showed
> > that for e10s-multi we could probably go up to 4 content processes
> without
> > blowing it out too badly, but 8 would be a major problem.
> ...
> > Nick
> >
> >
> >
> > On Sat, Mar 4, 2017 at 11:15 AM, Nicholas Nethercote <
> n.neth...@gmail.com
> > > wrote:
> >
> > > Hi,
> > >
> > > I want to understand all the different processes that we can and will
> have
> > > in Firefox. Here's a list I constructed off the top of my head.
> > >
> > > - main process
> > >
> > > - content process(es): 1 on release for most users; 2 on Nightly
> > >
> > > - plugin process: just for Flash now?
> > >
> > > - gfx compositor process (bug 1264543, in Fx53)
> > >
> > > - file:// URL access process (bug 1147911, in Fx53)
> > >
> > > IIRC there was a proposal for a thumbnail generation process a while
> back
> > > but judging by bug 1187441 that was scrapped.
> > >
> > > Do I have any of these details wrong? Have I missed any?
> > >
> > > Thanks.
> > >
> > > Nick
> > >
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: All the processes

[Eric Rahm]

It should be pretty easy to measure at a high level with ATSY [1], but I
agree coordination on overall memory requirements before adding new
processes would be useful.

For more detailed breakdowns of memory usage we can depend on about:memory
reports for content processes, we added support for memory reports from the
GPU process recently [2], I'm not sure about the other processes; if
they're just dedicated content processes then it should just work, if not
they could probably piggy back off what the GPU process did.

I assume WebRenderer will have it's own process, but maybe that just gets
lumped in with the GPU process.

-e

[1] https://github.com/EricRahm/atsy
[2] https://bug623317.bugzilla.mozilla.org/show_bug.cgi?id=1321492

On Mon, Mar 6, 2017 at 2:42 PM, Nicholas Nethercote 
wrote:

> On Tue, Mar 7, 2017 at 9:22 AM, Ben Kelly  wrote:
>
> > These measurements are for full content processes.  Many of the processes
> > in the above do not need all the chrome script we load in content
> processes
> > today.
> >
>
> That's good to know. But it would still be good to get measurements of
> these slimmer processes, and check that they don't contain unnecessary
> stuff, and decide what coordination is necessary between the different
> process types (as kmag suggested elsewhere in the thread).
>
> Nick
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Heads up: archive.m.o is no longer updating tinderbox builds, AWSY is effectively dead

[Eric Rahm]

I filed bug 1344936 [1] for the archive.m.o issue and updated mozdownload's
"add support for taskcluster" bug [2].

-e

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1344936
[2] https://github.com/mozilla/mozdownload/issues/365

On Mon, Mar 6, 2017 at 1:33 PM, Henrik Skupin <hsku...@mozilla.com> wrote:

> Eric Rahm wrote on 06/03/2017 22:17:
>
> > I'm unaware of a bug for this decision, but https://archive.mozilla.org
> > stopped adding tinderbox builds back on January 18th.
>
> I haven't heard about that myself. So I'm curious if that is a
> regression by a change in Releng automation? I would suggest you simply
> file a bug in RelEng:General Automation in Bugzilla. Please put me on CC.
>
> > AWSY also depends on mozdownload [3] which will need to be fixed to
> handle
> > not being able to download from archive.m.o as well. I don't believe
> > mozdownload has an active owner at this point.
>
> I'm still the owner of this tool / project, but as said above I haven't
> been made aware of a change here. So if we do not publish tinderbox
> builds to archive.mozilla.org anymore - and if that is intended - we
> indeed would need another way to download this builds. And that most
> likely by adding support for Taskcluster.
>
> Please also note that nothing of my projects including Mozmill-CI is
> using Tinderbox builds. So if something is broken in the future please
> file issues for those tools / packages which are not working as expected.
>
> Cheers,
>
> --
> Henrik Skupin
> Senior Software Engineer
> Mozilla Corporation
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Doxygen output?

[Eric Rahm]

I was just thinking about this as I probably wouldn't use the standard
doxygen output much, but can definitely see the usefulness when integrated
into our code indexers which I use quite often. Most useful to me would be:

   - @param doc when hovering over a param (in function, or when calling a
   function)
  - bonus points for annotating the type
  - @class doc when hovering over a class (fallback to base class would
   be nice)
   - @method doc when hovering over a function definition or call

Integrating IDL docs would be nice as well.

-e

On Tue, Feb 21, 2017 at 12:13 PM, Bill McCloskey 
wrote:

> I've been thinking about how to integrate documentation into Searchfox. One
> obvious thing is to allow it to display Markdown files and
> reStructuredText. I wonder if it could do something useful with Doxygen
> comments though? Is this something people would be interested in?
>
> On Tue, Feb 21, 2017 at 11:53 AM, Ted Mielczarek 
> wrote:
>
> > We have auto-generated docs using Sphinx on ReadTheDocs[1]. If someone
> > was motivated, it looks like there does exist code[2] to bridge doxygen
> > docs into Sphinx, so it should be possible to get those docs into the
> > existing RTD setup. There are even docs on RTD[3] for how to add new
> > docs!
> >
> > -Ted
> >
> > 1. http://gecko.readthedocs.io/en/latest/
> > 2. https://breathe.readthedocs.io/en/latest/
> > 3. http://gecko.readthedocs.io/en/latest/#adding-documentation
> >
> >
> > On Mon, Feb 20, 2017, at 11:38 AM, Milan Sreckovic wrote:
> > > Not being kept up to date as far as I know.  My extraction is four
> years
> > > out of date (e.g.,
> > > https://people-mozilla.org/~msreckovic/Extracted/
> > MozillaCentral/html/annotated.html)
> > > and as you noted, Benoit's page is no longer.
> > >
> > > The code used to create it is here:
> > > https://github.com/bgirard/doxygen-mozilla
> > >
> > >
> > > On 20-Feb-17 2:05, Henri Sivonen wrote:
> > > > Our comments mostly try to follow the Doxygen format, and MDN says
> > > > that the documentation team has a tool for importing
> Doxygen-formatted
> > > > IDL comments into MDN articles.
> > > >
> > > > Other than that, is Doxygen output from m-c input being published
> > anywhere?
> > > >
> > > > https://people-mozilla.org/~bgirard/doxygen/gfx/ is 404 these days.
> > > >
> > >
> > > --
> > > - Milan (mi...@mozilla.com)
> > >
> > > ___
> > > dev-platform mailing list
> > > dev-platform@lists.mozilla.org
> > > https://lists.mozilla.org/listinfo/dev-platform
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> >
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Frame Pointer Omission Unconditionally Disabled on Win32

[Eric Rahm]

As of landing bug 1322735 [1], we now unconditionally disable frame pointer
omission (FPO) on all win32 builds. FPO was already disabled on
nightly/aurora builds as a side-effect of having profiling enabled, but
will now be disabled in beta (and release eventually) after the next
uplift.

Please note: This does not mean profiling is always enabled, that's still
controlled by the --enable-profiling switch.

The main motivation for this is to get usable crash stacks via telemetry
(see bug 1280477 [2])  without the need to have users submit full crash
reports. Comment 0 in bug 1322735 [1] provides further details and analysis.

One thing we want to look out for are talos regressions once this hits
beta, after initial testing on try we determined the few minor regressions
are an acceptable trade off [3].

As a nice side effect we were able to remove the |MOZ_STACKWALKING| define.
Please don't use that anymore.

-e

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1322735
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1280477
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1322735#c12
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Several leak failures have slipped passed continuous integration

[Eric Rahm]

Also note this has happened before. mccr8 was looking into similar
leak-checking-is-totally-busted-but-nobody-noticed issues a few years ago
in https://bugzilla.mozilla.org/show_bug.cgi?id=1045316

Glad to hear you're looking into end-to-end tests!

-e

On Thu, Dec 29, 2016 at 8:37 AM, Andrew Halberstadt <
ahalberst...@mozilla.com> wrote:

> Over the holidays, we noticed that leaks in mochitest and reftest were not
> turning jobs orange, and that the test harnesses had been running in that
> state for quite some time. During this time several leak related test
> failures have landed, which can be tracked with this dependency tree:
> https://bugzilla.mozilla.org/showdependencytree.cgi?id=
> 1325148_resolved=0
>
> The issue causing jobs to remain green has been fixed, however the known
> leak regressions had to be whitelisted to allow this fix to land. So while
> future leak regressions will properly fail, the existing ones (in the
> dependency tree) still need to be fixed. For mochitest, the whitelist can
> be found here:
> https://dxr.mozilla.org/mozilla-central/source/
> testing/mochitest/runtests.py#2218
>
> Other than that, leak checking is only disabled on linux crashtests.
>
> Please take a quick look to see if there is a leak in a component for
> which you could help out. I will continue to help with triage and bisection
> for the remaining issues until they are all fixed. Also big thanks to all
> the people who are currently working on a fix or have already landed a fix.
>
> Read on only if you are interested in the details.
>
>
>
> *Why wasn't this caught earlier? *
> The short answer to this question is that we do not have adequate testing
> of our CI.
>
> The problem happened at the intersection between mozharness and the test
> harnesses. Basically a change in mozharness exposed a latent bug in the
> test harnesses, and was able to land because it appeared as if nothing went
> wrong. Catching errors like this is tricky because regular unit tests would
> not have detected it either. It requires integration tests of the CI system
> as a whole (spanning test harnesses, mozharness and buildbot/taskcluster).
>
>
> *How will we prevent this in the future?*
>
> Historically, integration testing our test harnesses has been a hard
> problem. However with recent work in taskcluster, python tests and some
> refactoring on the build frontend, I believe there is a path forward that
> will allow us to stand up this kind of test. I will commit some of my time
> to fix this and hope to have *something* running that would have caught
> this by the end of Q1.
>
> I would also like to stand up a test harness designed to test command line
> applications in CI, which would provide another avenue for writing test
> harness unit and integration tests. Bug 1311991
>  will track this
> work.
>
> It is important that developers are able to trust our tests, and when bugs
> like this happen, that trust is eroded. For that I'd like to apologize, and
> express my hope that this will be the last time a major test result bug
> like this happens again. At the very least, we need to have the capability
> of adding a regression test when a bug like this happens in the future.
>
> Thanks for your help and understanding.
> - Andrew
>
> ___
> firefox-dev mailing list
> firefox-...@mozilla.org
> https://mail.mozilla.org/listinfo/firefox-dev
>
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Introducing mozilla::Result for better error handling

[Eric Rahm]

The key point for me is that we're hiding the return. I'm fine with the
more verbose 
explicitly-return-and-make-it-easier-for-the-reviewer-to-call-out-issues
form. I understand this is going to have differing opinions -- I think
there's merit to more concise code -- but for the things I look at I much
prefer an explicit return. It makes both the writer and reviewer think
about the consequences. If MOZ_TRY just release asserted on error I
wouldn't have an issue.

On Wed, Dec 21, 2016 at 10:59 PM, Kris Maglione <kmagli...@mozilla.com>
wrote:

> On Wed, Dec 21, 2016 at 10:53:45PM -0800, Eric Rahm wrote:
>
>> I like the idea of pulling in some Rusty concepts, but I'm concerned about
>> adding yet another early return macro -- absolutely no arguments on the
>> new
>> type, just the |MOZ_TRY| macro. In practice these have lead to security
>> issues (UAF, etc) and memory leaks in the C++ world (I'm looking at you
>> NS_ENSURE). These aren't hypothetical issues, I've run into them both
>> working on memshrink and sec issues in Firefox.
>>
>
> We run into the same issues without those macros. The only real difference
> is that when you have to follow every call with:
>
>  if (NS_WARN_IF(NS_FAILED(rv))) {
>return rv;
>  }
>
> It's much easier to lose track of the allocation you did four  calls ago
> that's now 20 lines away.
>
>
> On Wed, Dec 21, 2016 at 9:53 AM, Ted Mielczarek <t...@mielczarek.org>
>> wrote:
>>
>> On Wed, Dec 21, 2016, at 12:30 PM, Jason Orendorff wrote:
>>> > The implicit conversion solves a real problem. Imagine these two
>>> > operations
>>> > have two different error types:
>>> >
>>> > MOZ_TRY(JS_DoFirstThing()); // JS::Error&
>>> > MOZ_TRY(mozilla::pkix::DoSecondThing()); // pkix::Error
>>> >
>>> > We don't want our error-handling scheme getting in the way of using
>>> them
>>> > together. So we need a way of unifying the two error types: a shared
>>> base
>>> > class, perhaps, or a variant.
>>> >
>>> > Experience with Rust says that MOZ_TRY definitely needs to address this
>>> > problem somehow. C++ implicit conversion is just one way to go; we can
>>> > discuss alternatives in the bug.
>>>
>>> The `try` macro in Rust will auto-convert error types that implement
>>> `Into`, AIUI, but that's not automatic for all error types. I haven't
>>> tried it, but I have seen multiple recommendations for the `error_chain`
>>> crate to make this smoother:
>>> https://docs.rs/error-chain/0.7.1/error_chain/
>>>
>>> It's basically just boilerplate to implement conversions from other
>>> error types. I wouldn't be surprised if something like that percolates
>>> into the Rust standard library at some point.
>>>
>>> -Ted
>>> ___
>>> dev-platform mailing list
>>> dev-platform@lists.mozilla.org
>>> https://lists.mozilla.org/listinfo/dev-platform
>>>
>>> ___
>> dev-platform mailing list
>> dev-platform@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform
>>
>
> --
> Kris Maglione
> Firefox Add-ons Engineer
> Mozilla Corporation
>
> It's always good to take an orthogonal view of something.  It develops
> ideas.
> --Ken Thompson
>
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Introducing mozilla::Result for better error handling

[Eric Rahm]

I like the idea of pulling in some Rusty concepts, but I'm concerned about
adding yet another early return macro -- absolutely no arguments on the new
type, just the |MOZ_TRY| macro. In practice these have lead to security
issues (UAF, etc) and memory leaks in the C++ world (I'm looking at you
NS_ENSURE). These aren't hypothetical issues, I've run into them both
working on memshrink and sec issues in Firefox.

A reasonable defense is that we should RAII-all-the-things, and believe me
I get it, but we're working on a 20 year old C++ codebase at this point and
we have to deal with what we have. If we really want the benefits of Rust
we should be writing components in Rust, not trying to make C++ look like
Rust, but you know, without the memory safety guarantees.

-e

On Wed, Dec 21, 2016 at 9:53 AM, Ted Mielczarek  wrote:

> On Wed, Dec 21, 2016, at 12:30 PM, Jason Orendorff wrote:
> > The implicit conversion solves a real problem. Imagine these two
> > operations
> > have two different error types:
> >
> > MOZ_TRY(JS_DoFirstThing()); // JS::Error&
> > MOZ_TRY(mozilla::pkix::DoSecondThing()); // pkix::Error
> >
> > We don't want our error-handling scheme getting in the way of using them
> > together. So we need a way of unifying the two error types: a shared base
> > class, perhaps, or a variant.
> >
> > Experience with Rust says that MOZ_TRY definitely needs to address this
> > problem somehow. C++ implicit conversion is just one way to go; we can
> > discuss alternatives in the bug.
>
> The `try` macro in Rust will auto-convert error types that implement
> `Into`, AIUI, but that's not automatic for all error types. I haven't
> tried it, but I have seen multiple recommendations for the `error_chain`
> crate to make this smoother:
> https://docs.rs/error-chain/0.7.1/error_chain/
>
> It's basically just boilerplate to implement conversions from other
> error types. I wouldn't be surprised if something like that percolates
> into the Rust standard library at some point.
>
> -Ted
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

nsISupportsArray and friends are deprecated, slated for removal in 55

[Eric Rahm]

nsISupportsArray, nsICollection, nsIEnumerator, nsIBidirectionalEnumerator
have all been marked as deprecated in their IDL declarations as of Firefox
52 (currently dev edition).

This is not a new thing -- they've been semi-deprecated for 12 years [1] --
we just never fully removed them.

Over the years there's been a ton of work to remove usages of the
interfaces. I recently did a fair amount of work in bug 792209 [2] to
finish off getting rid of all remaining usage in mozilla-central and to
make our alternative, nsIArray, behave like nsISupportsArray in order to
make it easier to transition add-ons.

*What does this mean?*

Not that much, just please don't use those interfaces. You should see a
slew of warnings if you do (in C++ at least).

*How can I help?*

*comm-central* - mailnews/calendar still has some usage [3], if you have
some spare cycles to help out I'm sure they would appreciate it. If we
can't get rid of the usage by release 55 I still plan on removing the IDL
files from mozilla-central, it's possible we can move the definitions to
comm-central instead.

*add-ons* - Are you an add-on dev? Do you know an add-on dev? Transition
your code from using those interfaces or encourage the developer of your
favorite add-on to do so.


*What do I use instead?*

*nsIArray* and *nsIMutableArray* along with nsISimpleEnumerator are the
recommended replacements. For the most part they are drop-in compatible
with nsISupportsArray.

In C++ code if you need a new mutable array just use:

> nsCOMPtr arr = nsArray::Create();
>

In JavaScript:

> let arr = Cc["@mozilla.org/array;1"].createInstance(Ci.nsIMutableArray);


-e

[1]
https://groups.google.com/d/msg/netscape.public.mozilla.xpcom/RDWISpaMqAY/FUCS7G9ZKKIJ
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=792209
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=394167
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Please do not add any new CppUnitTests/GeckoCppUnitTests

[Eric Rahm]

Would that be a fourth copy in the tree at this point?

-e

On Wed, Nov 9, 2016 at 4:17 PM, Mike Hommey  wrote:

> On Thu, Nov 10, 2016 at 11:04:05AM +1100, Nicholas Nethercote wrote:
> > On Thu, Nov 10, 2016 at 10:00 AM, Mike Hommey  wrote:
> >
> > >
> > > CppUnitTests are fine to keep.
> >
> >
> > Having said that, IMO it is desirable to convert CppUnitTests to gtests
> > where possible. Every CppUnitTest has to provide some basic
> check/pass/fail
> > infrastructure, and many of them provide their own. Converting to gtest
> > provides consistency and usually ends up making tests shorter. E.g. a lot
> > of code like this:
> >
> >   if (!Foo(bar)) {
> > printf("Foo failed");
> > return false;
> >   }
> >
> > becomes this:
> >
> >   ASSERT_TRUE(Foo(bar)) << "Foo failed";
> >
> > The tests in mfbt/tests/ are good candidates for conversion
>
> Arguably, the tests in mfbt/tests are good examples of what not to touch:
> they are valuable to run (and are built) in e.g. standalone js builds,
> which don't have libxul-gtests.
>
> The issue of C++ test harness is however real, but it's not a matter of
> a simple conversion: it's a matter of having a non-libxul-gtest gtest
> C++ test harness.
>
> Mike
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Some recent crash-stats improvements

[Eric Rahm]

Is there a reason you need permissions for the memory report? Don't we
anonymize it?

-e

On Mon, Oct 10, 2016 at 8:34 AM, Benjamin Smedberg 
wrote:

> Yes, downloading raw minidumps and memory reports requires the PII
> permission in crash-stats. This can be obtained by Mozilla employees with
> the assent of your manager, although I'm not sure now what the correct
> bugzilla component is.
>
> --BDS
>
> On Mon, Oct 10, 2016 at 6:23 PM, Ben Kelly  wrote:
>
> > Does the "raw dump" tab support require special permissions?  I don't see
> > any way to download the memory report from:
> >
> > https://crash-stats.mozilla.com/report/index/1a572047-
> > ac64-4add-a82f-a31512161004#tab-rawdump
> >
> > On Sun, Oct 9, 2016 at 7:51 PM, Nicholas Nethercote <
> > n.netherc...@gmail.com>
> > wrote:
> >
> > > Greetings,
> > >
> > > crash-stats.mozilla.org has had some recent improvements that are
> > > worth highlighting.
> > >
> > > - There is new documentation on how to search through crash reports:
> > > https://developer.mozilla.org/en-US/docs/A_guide_to_
> > > searching_crash_reports.
> > > You might think you already know how to do this, but crash-stats'
> > > functionality for *grouping* search results is powerful and easy to
> > > overlook, so this page is worth reading to make sure you understand it
> > > fully. This documentation is linked to from the Super Search page
> > > within crash-stats.
> > >
> > > - Some crash reports contain memory reports. (The
> > > "ContainsMemoryReport" field is set in that case.) It used to be
> > > difficult to download this memory report, but thanks to Peter
> > > Bengtsson it's now much easier -- when a memory report is present,
> > > there's now a link to it from the "Raw Dump" tab of an individual
> > > crash report. See
> > > https://crash-stats.mozilla.com/report/index/1a572047-
> > > ac64-4add-a82f-a31512161004#tab-rawdump
> > > for an example.
> > >
> > > - Also, something that is not in crash-stats but is closely related:
> > > Marco Castellucio's new crash tools at
> > > https://mozilla.github.io/stab-crashes/. There are several tools, but
> > > probably the most widely-applicable one is
> > > https://mozilla.github.io/stab-crashes/correlations.html, which finds
> > > correlations for a particular crash signature. It works much better
> > > than the existing correlations tool within crash-stats. If you have a
> > > crash for which the cause is not obvious, this tool can be enormously
> > > helpful -- e.g. bug 1307029 is for a generic-looking crash signature
> > > that turns out to be highly correlated with a particular extension.
> > >
> > > Nick
> > > ___
> > > dev-platform mailing list
> > > dev-platform@lists.mozilla.org
> > > https://lists.mozilla.org/listinfo/dev-platform
> > >
> > ___
> > dev-platform mailing list
> > dev-platform@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-platform
> >
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: MXR permanently offline, please transition to DXR

[Eric Rahm]

On Monday, July 11, 2016 at 6:45:29 AM UTC-7, Matthew N. wrote:
> On Fri, Jul 8, 2016 at 12:27 PM, Lawrence Mandel 
> wrote:
> 
> > We do in the case of 3rd party software referencing files from MXR (and
> > I'm told there is a lot of this).
> >
> 
> ​That's an existing problem which is orthogonal to the MXR decommissioning
> so that doesn't need to be solved now or block better solutions to the
> problem at hand.
> 
> IMO we should redirect mxr.mozilla.org + lxr.mozilla.org to ​dxr.mozilla.org's
> server and have some rewrite rules to fix the cases where the URL needs to
> change to retrieve the equivalent content.
> 
> ​Rewriting inbound links is addressing the problem at the wrong place IMO
> since there are so many links you won't be able to fix and there's not a
> good reason to break them.
> 
> 
> > We also can't guarantee that MXR URLs will direct to the right place in
> > DXR. There is likely a balance to be struck here with highly referenced
> > files from 3rd party software getting an interstitial page and other files
> > not getting the page. Let's start with getting the page with the redirect
> > in place and then iterate from there as required.
> >
> > Lawrence
> >
> >
> > On Fri, Jul 8, 2016 at 3:24 PM, Bobby Holley 
> > wrote:
> >
> >> Can we skip the interstitial page and make the notice (if any) more
> >> unobtrusive somehow? There are tons of mxr links all over the place, and
> >> many of them are immutable. We don't gain anything by informing the viewer
> >> about their obsolescence instead of showing them the content they want.
> >>
> >> On Fri, Jul 8, 2016 at 12:20 PM, Lawrence Mandel 
> >> wrote:
> >>
> >>> dev-platform was not included on my response below. Looping back in to
> >>> this
> >>> fork of the thread.
> >>>
> >>> On Fri, Jul 8, 2016 at 10:55 AM, Lawrence Mandel 
> >>> wrote:
> >>>
> >>> > Sorry Dao. I have seen some responses. Maybe they were off list. We're
> >>> > working on details now. I'm going to get someone to put the redirects
> >>> in
> >>> > place, likely with an interstitial page advising that MXR has been
> >>> > decommissioned, by next week.
> >>> >
> >>> > Lawrence
> >>> >
> >>> >
> >>> > On Friday, 8 July 2016, Dão Gottwald  wrote:
> >>> >
> >>> >> Why has nobody responded to the requests for a short-term fix for MXR
> >>> >> URLs for more than a week? Are the people responsible for MXR not in
> >>> this
> >>> >> list?
> >>> >>
> >>> >> 2016-07-07 18:23 GMT+02:00 Eric Shepherd :
> >>> >>
> >>> >>> We have tons of mxr links all through MDN, fwiw. I am updating the
> >>> >>> macros that generate them, but odds are very good these links will be
> >>> >>> around for a good while.
> >>> >>>
> >>> >>>
> >>> >>> That would be perfectly fine for my purposes, I expect, as long as it
> >>> >>> dealt with the relevant mxr features.  What I want is for links to
> >>> possibly
> >>> >>> specific lines of possibly specific revisions of specific files to
> >>> work.
> >>> >>> Ideally with the highlighting bits too.
> >>> >>>
> >>> >>>
> >>> >>> --
> >>> >>>
> >>> >>> Eric Shepherd
> >>> >>> Senior Technical Writer
> >>> >>> Mozilla Developer Network 
> >>> >>> Blog: https://www.bitstampede.com/
> >>> >>> Twitter: http://twitter.com/sheppy
> >>> >>> Doodle: http://doodle.com/the.sheppy
> >>> >>>
> >>> >>>
> >>> >>> ___
> >>> >>> firefox-dev mailing list
> >>> >>> firefox-...@mozilla.org
> >>> >>> https://mail.mozilla.org/listinfo/firefox-dev
> >>> >>>
> >>> >>>
> >>> >>
> >>> ___
> >>> dev-platform mailing list
> >>> dev-platform@lists.mozilla.org
> >>> https://lists.mozilla.org/listinfo/dev-platform
> >>>
> >>
> >>
> >
> > ___
> > firefox-dev mailing list
> > firefox-...@mozilla.org
> > https://mail.mozilla.org/listinfo/firefox-dev
> >
> >

Alright I filed bug 1286076 [1] for tracking the "redirect MXR" discussion and 
bug 1286079 [2] for the "adding multi-line highlight to hgweb" discussion. I 
don't think I saw anything else actionable in the thread (the text contrast 
issue has already been fixed), but if you have specific requests I'd suggest 
filing a bug [3,4] in addition to any discussion here.

-e

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1286076
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1286079
[3] https://bugzilla.mozilla.org/enter_bug.cgi?product=Webtools=DXR
[4] 
https://bugzilla.mozilla.org/enter_bug.cgi?product=Developer%20Services=Mercurial%3A%20hg.mozilla.org
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: MXR permanently offline, please transition to DXR

[Eric Rahm]

While mxr being retired is unfortunate, at this point would it not make
more sense to just file bugs against dxr for features you would like to be
added?

It's not clear to me after this lengthy discussion if anything actionable
has been accomplished.

-e

On Fri, Jul 8, 2016 at 7:57 PM, Boris Zbarsky  wrote:

> On 7/8/16 7:25 PM, Gerald Squelart wrote:
>
>> The annotate (aka blame) functionality of hg.mozilla.org can point at
>> one line
>>
>
> Yes, I know.  What it can't do is highlight some set of lines containing
> more than one line.  Think things like:
>
>   http://mxr.mozilla.org/whatever-file?mark=10-20,17,25#8
>
> which highlights lines 10-20, 17, and 25, and scrolls to line 8.  This is
> very useful when pointing people at code.
>
> -Boris
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

How do we measure active users on a given architecture?

[Eric Rahm]

I've seen statements such as "0.04% Firefox users don't have SSE"*. What
I'd like to know is if when we make these measurements we also ascertain
whether or not those users are on the most recent release.

There's often pushback, which I think is certainly reasonable, related to
abandoning users or maintaining an ESR to support said users that we're
abandoning. So my question is: How many of these users are actively
updating? I think this would be useful in making decisions on what support
to deprecate.

-e

*not a direct quote, probably not accurate
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: All about crashes

[Eric Rahm]

Thanks for putting this together!

It would be nice to see some consolidated details on how to investigate
crashes, ie loading minidumps in Visual Studio, interpreting results,
figuring out when VS is lying to you and what disassembly to look at. I
think there's some great institutional knowledge here that would be useful
to gather. Many of our developers don't use Windows regularly, so having
details on how to get going quickly would be very much appreciated.

Details on using rr to debug crashes would certainly be nice.

Additionally it would be great to see information on handling OOMs (large
and small) as those are our top crashers, and if anything I think project
uptime should be focusing on mitigating them. Fixing null derefs for a few
hundred users is nice, but fixing OOMs for tens of thousands of users is
even better!

-e

On Tue, May 24, 2016 at 9:59 PM, Boris Zbarsky  wrote:

> On 5/25/16 12:05 AM, Tobias B. Besemer wrote:
>
>> Why are there so many crashes on Windows and so less on Linux ???
>>
>
> Mostly because there are so many more people running Firefox on Windows
> than on Linux...
>
> -Boris
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Updating 32-bit Windows users to 64-bit Windows builds?

[Eric Rahm]

I filed bug 1274659 [1] to track this proposal and attempted to summarize
the issues brought up. Please add any technical comments and blocking bugs
there.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1274659

-e

On Fri, May 20, 2016 at 5:56 AM, Tobias B. Besemer <
tobias.bese...@googlemail.com> wrote:

> Am Freitag, 20. Mai 2016 01:48:24 UTC+2 schrieb Robert Strong:
> > On Thu, May 19, 2016 at 3:18 PM, Tobias B. Besemer wrote:
> >
> > > Am Freitag, 13. Mai 2016 22:41:01 UTC+2 schrieb Benjamin Smedberg:
> > > > We have considered this, but in the grand rollout plans for 64-bit
> > > Firefox
> > > > it's low on the list. We're still dealing with Flash
> > > sandboxing/functional
> > > > regressions as a blocker for wider rollout, and the next step is
> probably
> > > > to progressively roll out win64 to new users before we consider
> anything
> > > > for existing users.
> > > >
> > > > This will be much easier now that we have widevine and are dropping
> > > > npapi/silverlight, but addon compat is also a concern and we wanted
> to
> > > > partly wait for webextensions before pushing more on this.
> > > >
> > > > --BDS
> > >
> > > Sounds like a plan for me!
> > > Maybe there can be a ship of a installer that include 32bit & 64bit?
> > > Or at least have one web-installer for both versions?
> > > Also giving the user the change to make a easy upgrade from 32bit to
> 64bit
> > > with the offline-installer would be nice and a good test-drive for a
> future
> > > auto-update...
> > >
> > The installer does not equal auto-update. Two separate things entirely.
> > Download size for a combined installer is not something we want to do to
> > people on slow network connection but the auto selection via the stub
> > installer is planned though no completion date yet due to other work
> having
> > priority.
>
> The idea was to test the upgrade from 32bit to 64bit first with the
> offline installer because it should effect less people and would be maybe a
> good test for all the routines/logic behind it like e.g. uninstall
> something, moving files, or something like this...
>
> If not to much work, I would prefer to have one 32bit/64bit-installer for
> people who don't know the difference... (as default download.)
> Single Just-32bit/64bit-installer can persist for people who know for what
> they have to looking for... (AFAICR other project did/do the same.) (At
> least with just-English and multi-lang installers...)
>
> As I didn't knew how Mozilla will handle the switch... if - like by IE -
> there will be 32bit/64bit parallel, or like Chrome do it, just one
> version... I installed from each channel both version on my system and
> created a bunch of icons for it, because the version overwrite ATM the
> icons from each other...
> I guess that a lot of people have the almost same scenario (both
> versions), but by mistake and don't realize it!
> So a routine (first in offline installer) in the 64bit version that check
> if a (old) 32bit version exist too on the system and when, then de-install
> it while install/update the 64bit version would be (IMHO) nice.
> (Can test this and make QA.)
>
> Also I would like to see a error msg in future (or at least a big warning)
> if a user try to use the 32bit installer on a 64bit system.
>
> AFAIK there is also no MozillaMaintenanceService as 64bit now...
>
> ...and the MozillaMaintenanceService should also block to install a 32bit
> version on a 64bit Win (even normally no-one use this installer manual) and
> uninstall 32bit if 64bit gets installed or updated.
>
> A long open wish from me (and I guess others, too) would be to see in
> future a multi-lang web-installer. Should also make things easier...
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Updating 32-bit Windows users to 64-bit Windows builds?

[Eric Rahm]

Last time I checked we saw something like a 35% increase in overhead on
AWSY going from 32-bit to 64-bit Firefox on 64-bit Windows, so yes there is
a significant impact.

On the other hand you no-longer run into the
OOM-because-of-address-space-exhaustion and
OOM-because-we-can't-find-a-one-meg-chunk-in-this-horribly-fragmented-heap
issues. In theory even if the physical memory usage gets too high we can
hope that things get paged out rather than OOMing.

So there's a reasonable argument for just upping everyone who can to 64-bit.

-e

On Thu, May 12, 2016 at 2:46 PM, Karl Tomlinson  wrote:

> Lawrence Mandel writes:
>
> > Do we need this criteria?
> >
> > RAM - Does it hurt to move an instance that has <4GB?
>
> Yes.  OOM will be more common with 64-bit builds on systems with
> less RAM because 64-bit builds use more memory.
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: MOZ_LOG_FILE and MOZ_LOG_MODULES are now the environment variables for logging

[Eric Rahm]

On Thursday, March 24, 2016 at 10:10:30 AM UTC-7, Justin Dolske wrote:
> On 3/24/16 9:54 AM, Ralph Giles wrote:
> > On Mon, Mar 21, 2016 at 9:13 AM, Honza Bambas  wrote:
> >
> >> tl;dr: Start migrating to use of MOZ_LOG_* instead of NSPR_LOG_*.  Don't
> >> worry about backward compatibility tho, when MOZ_LOG_* is not set, we
> >> fallback to NSPR_LOG_* values.
> >
> > Could this be an opportunity to shorten NSPR_LOG_MODULES to just MOZ_LOG?
> 
> +1 to MOZ_LOG. It's better than bad, it's good.
> 
> Justin

That might be confusing as there's also the |MOZ_LOG| macro. OTOH it might be 
easier to remember...

I don't feel particularly strongly about it, I'd r+ a patch if it came my way.

-e
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Presto: Comparing Firefox performance with other browsers (and e10s with non-e10s)

[Eric Rahm]

On Friday, February 12, 2016 at 9:41:02 AM UTC-8, Patrick Meenan wrote:
> > > For example with Firefox I would be interested in the RSS of the parent 
> > > process (firefox.exe) and the USS of the child processes 
> > > (plugin-container.exe). For Chrome it would be more along the lines of 
> > > the RSS of the main chrome process, and the USS of the 
> > > renderer/gpu/plugin processes (and probably the RSS of the nacl process 
> > > if that's still around).
> > 
> > I can grab a one-time snapshot of these at the end of the test/page load.  
> > Walking the process list was too expensive to do every 100ms (which is how 
> > often I collect CPU utilization).  Should be able to add it today and 
> > report both the parent RSS and sum(child_uss) as separate numbers.
> 
> Just pushed support in WPT for collecting memory stats at the end of a test: 
> https://github.com/WPO-Foundation/webpagetest/commit/629f48ea5c57be57b50e1f97942c98dface593b2
> 
> Sample result: http://www.webpagetest.org/xmlResult/160212_3Z_12D1/
> 
> Specifically:
> browser_process_count - Number of browser processes
> browser_main_memory_kb - Full working set of the "main" browser process in KB 
> (main for WPT is whatever is doing the network communications)
> browser_other_private_memory_kb - Sum of the private working sets for all 
> other browser processes (in KB)
> browser_working_set_kb - Both working set numbers combined

This is great, thanks for adding it. Getting memory usage on "real" sites 
should be quite interesting.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Memory Usage on Perfherder & Memory Reduction

[Eric Rahm]

On Tuesday, February 9, 2016 at 2:25:42 PM UTC-8, Mark Finkle wrote:
> Hi All,
> 
> Recently Geoff Brown landed an AWSY-like system [1] for tracking memory
> usage on Perfherder. This is awesome. It's one of my pinned tabs.
> 
> I was happy to see two recent "drops" in memory usage:
> 
> 1. A ~3% drop in "Resident Memory Tabs closed [+30s]", likely due to Bug
> 990916 which expires displayports
> https://treeherder.mozilla.org/perf.html#/graphs?series=[mozilla-inbound,f9cdadf297fd409c043e8114ed0fa656334e7fad,1]=1454516622714.927,1454583882842.8733,181623181.32925725,250028978.43070653
> 
> 2. A ~2% drop across all memory tracking sometime on Feb8. Hard to pick a
> changeset, but the drop happened when inbound was merged to fx-team.
> https://treeherder.mozilla.org/perf.html#/graphs?series=%5Bmozilla-inbound,f9cdadf297fd409c043e8114ed0fa656334e7fad,1%5D
> 
> Great to see drops in memory usage!
> 
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1233220

It's great we're tracking memory usage on Fennec again! Just to be clear, this 
data is android-only (correct me if I'm wrong). On the Linux desktop side we've 
started pushing data from AWSY to the treeherder staging instance. Hopefully 
that will get that into production shortly.

-e
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Presto: Comparing Firefox performance with other browsers (and e10s with non-e10s)

[Eric Rahm]

Really interesting project, is this currently Windows only? It would be great 
if we could get memory usage as well.

Also just to clarify, this is WPT that runs on webpagetest.org with code from 
https://github.com/WPO-Foundation/webpagetest?

-e
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Memory Usage on Perfherder & Memory Reduction

[Eric Rahm]

On Wednesday, February 10, 2016 at 7:46:11 AM UTC-8, William Lachance wrote:
> Incidentally, the automatic regression detection dashboard has been 
> coming together recently with Perfherder, which should let you track 
> such things as this much more easily (as well as providing a convenient 
> interface for filing bugs). For example, you can see all recently 
> detected regressions and improvements in memory usage here:
> 
> https://treeherder.allizom.org/perf.html#/alerts?status=-1=4
> 
> (there's some false positives in there I know, we could probably do with 
> some slightly better regression-detection code...)
> 
> More updates soon.
> 
> Will
> 

That dashboard looks great, it'll be interesting to see how reliable the AWSY 
data is for generating automatic regression detection in the long run.

-e
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Presto: Comparing Firefox performance with other browsers (and e10s with non-e10s)

[Eric Rahm]

On Thursday, February 11, 2016 at 5:03:05 PM UTC-8, Patrick Meenan wrote:
> "Memory Usage" is  complicated.  Specially when you try to compare 
> different architectures.

Sure, but this is all Windows for desktop at least.

> Working set? Virtual memory? Accounting for shared pages, etc.

Working set (RSS) and private working set (USS) are the most interesting 
numbers. This gets tricky with multi-process setups, but a reasonable baseline 
I've been looking at is |total_memory = parent_rss + sum(child_uss)|

For example with Firefox I would be interested in the RSS of the parent process 
(firefox.exe) and the USS of the child processes (plugin-container.exe). For 
Chrome it would be more along the lines of the RSS of the main chrome process, 
and the USS of the renderer/gpu/plugin processes (and probably the RSS of the 
nacl process if that's still around).

> Optimizing for the wrong thing can have negative impacts (like optimizing for 
> the working set displayed in task manager is easy by forcing a process to 
> page out periodically but it's artificial and not good for anybody).

Artificial optimizations are an unfortunate side effect of every benchmark 
(particularly in js-land), I'm not sure it's our place to not measure something 
because we think people might game it. 

> WebPageTest used to track it at one point but the data wasn't actually useful 
> so I removed it. If anyone has suggestions on how to do it in a useful way 
> I'd be happy to add it.

See above.

-e
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Proposal to stop revving UUIDs when changing XPIDL interfaces

[Eric Rahm]

On Thursday, January 28, 2016 at 2:56:19 PM UTC-8, Ehsan Akhgari wrote:
> 10 days and no objections.  This is now the new rule!  Please stop updating
> UUIDs when changing XPIDL interfaces.
> 
> On Fri, Jan 15, 2016 at 10:58 AM, Ehsan Akhgari 
> wrote:
> 
> > Historically we have enforced updating the XPIDL interface UUIDs when you
> > made any changes to it.  This was needed because of two reasons:
> >
> > * Backwards compatibility with binary extensions.  Since many changes to
> > XPIDL interfaces caused the underlying v-table layout to change, revving
> > the UUID enabled previously compiled extensions to fail getting the
> > interface through QueryInterface() in the first place, preventing crashes
> > when they try to use the interface.
> >
> > * Incremental builds.  Our build system used to not repack the compiled
> > XPT file unless it detected a change in the UUID, which would manifest as
> > weird issues when you landed code changing an interface without changing
> > its UUID, in that in incremental builds the XPT file would be outdated, but
> > in clobbered builds it would be correct.
> >
> > We have created Mercurial hooks that enforce a UUID change when an idl
> > file is touched because of these requirements.
> >
> > Ever since Firefox 41, we have stopped supporting binary components in
> > extensions, so the first reason doesn't apply any more.  And since
> > yesterday I have fixed bug 977464 which fixes the second issue.  So as far
> > as I can tell, there is no reason to keep revving UUIDs any more. Therefore
> > I would like to propose that we should remove the Mercurial hook (bug
> > 1170718) and relax this requirement on trunk, and let this ride the trains.
> >
> > Three points worth mentioning here.
> >
> > * Thunderbird still supports binary components in extensions.  In <
> > https://bugzilla.mozilla.org/show_bug.cgi?id=977464#c31> Kent said that
> > Thunderbird is OK with change.
> >
> > * My proposal has no bearing on whether changes to XPIDL interfaces needs
> > to be considered as part of the uplift approval process, as such changes
> > can still have an impact on JS extension compatibility. Therefore under my
> > proposal we'd reword the approval canned questionnaire on Bugzilla to talk
> > about changes to XPIDL interfaces in addition to string changes, in lieu of
> > mentioning UUID changes.
> >
> > * UUIDs are still the unique identifiers used in QueryInterface()
> > implementations and you'd still need to tag the interface with a UUID when
> > you create a new XPIDL interface.
> >
> > Please let me know if you have any questions or concerns.
> >
> > Cheers,
> > Ehsan
> >
> 
> 
> 
> -- 
> Ehsan

Have the reject-on-idl-change-but-no-uuid-change scripts been updated on the hg 
server?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Dynamic Logging

[Eric Rahm]

On Monday, January 25, 2016 at 11:06:15 AM UTC-8, Honza Bambas wrote:
> Nice idea although unfortunately incomplete.  On Windows this has no 
> effect at all.  We cannot use it for common users.  When you set the 
> pref, it logs only to the debug output that can be captured only with 
> visual studio or similar software being attached.  Not something to ask 
> for a common user.

I'm reasonably sure it logs to stderr, if there is an issue in the Windows 
implementation please file a bug and we'll get it fixed.

> We also need to figure out how to start writing to a file w/o a 
> restart.  I believe a harder part of this effort...

Valentin has a patch for dynamically specifying a log file in bug 1239686.

-e
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Dynamic Logging

[Eric Rahm]

By default we use |printf_stderr|, which prints to stderr (or logcat on
android/b2g). I presume we grab that in the test logs, but maybe I'm
mistaken?

-e

On Fri, Jan 8, 2016 at 6:32 PM, Bobby Holley <bobbyhol...@gmail.com> wrote:

> This is incredible - thank you for pushing this through Eric!
>
> In case the implications of this aren't clear to anyone: One big
> difficulty with debugging intermittent failures is that enabling logging
> for the relevant components can often be too expensive for a try run. The
> logs can consume hundreds of megabytes, and quickly hit the TreeHerder
> limits (at which point you get nothing). We have the ability to record and
> upload a log as a separate artifact, but that's rarely useful because it's
> difficult to correlate the NSPR log output with the TestRunner spew in the
> other file.
>
> When I was working on media stability, I debugged dozens of race
> conditions by adding hacky instrumentation to make PR_LOG invoke printf if
> a certain script-accessible bit was set, and then retriggering on try until
> the failure occurred. This work will make that much more straightforward to
> do.
>
> Eric, is there an option to make the NSPR log output go directly into the
> regular test output?
>
> On Fri, Jan 8, 2016 at 5:32 PM, Eric Rahm <er...@mozilla.com> wrote:
>
>> Hi Folks-
>>
>> With bug 1233881 <https://bugzilla.mozilla.org/show_bug.cgi?id=1233881>
>> we
>> landed the ability turn on logging via prefs.
>>
>> Lets say you have a log module "Foo", if you add a "logging.Foo" pref and
>> set it to "Debug" you will now see all output from the Foo log module that
>> is of Debug and higher importance.
>>
>> Why is this so cool? Well now you don't need to restart your browser to
>> enable logging [1]. You also don't have to set env vars to enable logging
>> [2].
>>
>> There is one caveat: if you don't use LazyLogModule and friends, you don't
>> get dynamic logging. So go update your loggers!
>>
>> -e
>>
>> [1] Okay, this only kind of works right now. You'll still need to set
>> NSPR_LOG_MODULES="anything_you_want" to see output. Bug 1174972
>> <https://bugzilla.mozilla.org/show_bug.cgi?id=1174972> will fix this.
>>
>> [2] If you care about messages during startup you will still need to set
>> the NSPR_LOG_MODULES env var. Unfortunately it takes time to load the pref
>> system, and then more time to load your profile.
>> ___
>> dev-platform mailing list
>> dev-platform@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform
>>
>
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Dynamic Logging

[Eric Rahm]

Hi Folks-

With bug 1233881  we
landed the ability turn on logging via prefs.

Lets say you have a log module "Foo", if you add a "logging.Foo" pref and
set it to "Debug" you will now see all output from the Foo log module that
is of Debug and higher importance.

Why is this so cool? Well now you don't need to restart your browser to
enable logging [1]. You also don't have to set env vars to enable logging
[2].

There is one caveat: if you don't use LazyLogModule and friends, you don't
get dynamic logging. So go update your loggers!

-e

[1] Okay, this only kind of works right now. You'll still need to set
NSPR_LOG_MODULES="anything_you_want" to see output. Bug 1174972
 will fix this.

[2] If you care about messages during startup you will still need to set
the NSPR_LOG_MODULES env var. Unfortunately it takes time to load the pref
system, and then more time to load your profile.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Dynamic Logging

[Eric Rahm]

Yes, although until bug 1234892 is fixed you'll have to restart the process
for changes to take effect.

On Fri, Jan 8, 2016 at 5:51 PM, Shih-Chiang Chien <sch...@mozilla.com>
wrote:

> Cool! Does it also work on content process?
>
> Best Regards,
> Shih-Chiang Chien
> Mozilla Taiwan
>
> On Fri, Jan 8, 2016 at 5:38 PM, Patrick McManus <mcma...@ducksong.com>
> wrote:
>
>> On Fri, Jan 8, 2016 at 8:32 PM, Eric Rahm <er...@mozilla.com> wrote:
>>
>> > Why is this so cool? Well now you don't need to restart your browser to
>> > enable logging [1]. You also don't have to set env vars to enable
>> logging
>> > [2].
>> >
>>
>>
>> epic! thank you.
>> ___
>> dev-platform mailing list
>> dev-platform@lists.mozilla.org
>> https://lists.mozilla.org/listinfo/dev-platform
>>
>
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

MemShrink: an ongoing initiative to reduce memory consumption

[Eric Rahm]

Hi folks-

The MemShrink project [1] has been around for several years. You may have
seen the [MemShrink] tag added to bugs and then later on given a priority
of P1, P2, or P3. This was the product of weekly (and then bi-weekly) video
meetings held to triage these unprioritized bugs and discuss all things
memory.

We would like to move to a more asynchronous, open, and inclusive system.
To support this goal a new mailing list, dev-memory [2], has been created
where we will have a weekly triage discussion. All other memory related
topics are welcome as well.

For all interested please sign up for the list [3], add a [MemShrink] tag
to memory related bugs you see, participate in the triage discussion, and
post to the list about memory related topics and concerns.

And as a reminder: there will continue to be the  #memshrink IRC channel.

-e

[1] https://wiki.mozilla.org/Performance/MemShrink
[2] https://groups.google.com/forum/#!forum/mozilla.dev.memory
[3] https://lists.mozilla.org/listinfo/dev-memory
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

LazyLogModule, a thread-safe replacement for declaring a PRLogModuleInfo

[Eric Rahm]

Hi All-

I recently landed a lazily-loaded log module class, LazyLogModule [1],  
suitable for declaring static references to log modules in a thread-safe way. 

Currently this class is opt-in and PRLogModuleInfo can still be used w/ 
MOZ_LOG. But be forewarned, as we move forward to a glorious world where log 
levels can be dynamically updated w/o restarting the browser, PRLogModuleInfo 
will be left behind and only LogModule instances will get the benefit.

Before:
===

  PRLogModuleInfo*
  GetMyLog()
  {
static PRLogModuleInfo* sLogModule = PR_NewLogModule("MyLog");
return sLogModule;
  }

  void Foo() { MOZ_LOG(GetMyLog(), ...); }

Now:


  LazyLogModule sMyLogModule("MyLog");

  void Foo() { MOZ_LOG(sMyLogModule, ...); }

The Future:
===

We have already converted xpcom over to using it [2] and are quite happy with 
how things turned out.

This is where you come in dear reader!

Please switch over your PRLogModuleInfo instances to LazyLogModule. I have a 
tracking bug [3] for the overall code base and have split out bugs for smaller 
chunks. If you intend to help out just go ahead and assign yourself one of 
those bugs!

Addendum on Thread Safety
=

There are some common mistakes that TSan runs are tripping over, such as:

// Not-quite-right use of static initialization
GetLog() {
  static* myLog = nullptr; // This is thread-safe on modern compilers
  if (!myLog)
myLog = ... // This is not.
}

// Global, initialize wherever it's used
static PRLogModuleInfo* myLog;

Foo::Foo() {
  if (!myLog)
myLog = ...
}

And to round it out, PR_NewLogModule is not thread-safe [4].

-e

[1] 
https://dxr.mozilla.org/mozilla-central/rev/fc706d376f0658e560a59c3dd520437b18e8c4a4/xpcom/base/Logging.h#100
[2] https://hg.mozilla.org/mozilla-central/rev/f9cf413cb3da
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=1219461
[4] https://bugzilla.mozilla.org/show_bug.cgi?id=1073578
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Removing "nsDebug" logger

[Eric Rahm]

On Monday, October 19, 2015 at 11:39:20 AM UTC-7, eric...@gmail.com wrote:
> I am removing the "nsDebug" logger [1] in bug 1215629 [2]. It was added some 
> 12 years ago and the purpose has seemingly been lost over the years.
> 
> If you happen to rely on this being available please let us know. If I don't 
> hear any strong objections in the next day I will land the removal patch.
> 
> A cursory search of m-c indicates it is not being used by any in-tree tools. 
> Fear not: even with the removal of the logger, warnings will maintain the 
> current behavior of being printed to stderr.
> 
> -e
> 
> [1] 
> https://dxr.mozilla.org/mozilla-central/rev/d1a89632277fbaaf470c90a35573776048988f2d/xpcom/base/nsDebugImpl.cpp#375
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1215629

Patch landed today: 
https://hg.mozilla.org/integration/mozilla-inbound/rev/3bc66762ac6ec54351a63f9d18cf4ef9f3d31a54

-e
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

The War on Warnings Redux

[Eric Rahm]

TL;DR - We have reduced the amount of runtime warnings during testing by 90%.

As of today we are down to 66K warnings during linux64 debug testing; this is 
down from 600K back in June. There are bugs on file for most of the remaining 
top offenders blocking tracking bug 765224 [1]. It's time to declare victory, 
or at least transition to a skirmish, or perhaps an extended conflict. It's 
been a fun ride, I've more than surpassed my goal of a sub 100K count and it's 
time for me to write some real code again.

Reducing the amount of warnings has made it *much* easier to weed out real bugs 
and many of the remaining warnings most likely *are* bugs.

This is where you can help out: take a look at the tracking bug (and the 
following list) and see if there is a warning in an area you can help out with. 
Even just triaging by adding a needinfo? for folks who could help out would be 
super helpful.

For those interested I have put up my (somewhat sketchy) set of scripts for 
parsing logs and counting warnings on github [2]. Contributions and feedback 
are welcome.

Going forward I can think of a few interesting projects for the automation 
inclined:
  - Setup some sort of daily warning analysis, bisect and send out nag emails
  - Add support for automatically filing bugs to the log spam scripts
  - Turn the tree red if warnings go over some threshold
  - Get to a warning free startup state, add tests that enforce no new warnings 
on startup

Again, please take a moment to go over the list, file more bugs and clean out 
unhelpful warnings so that we can focus on the real issues

Big thanks to smaug, bz, dholbert and everyone else who helped get patches up, 
reviewed and in general dealt with my incessant nagging.

-e

The final countdown:

TOP 40
==
   3997 [N] WARNING: Please do not use mouseenter/leave events in chrome. 
They are slower than mouseover/out!: '!nsContentUtils::IsChromeDoc(d)', file 
dom/events/EventListenerManager.cpp, line 370
   2523 [N] WARNING: '!mContentCache.CacheEditorRect(this, 
aIMENotification)', file widget/PuppetWidget.cpp, line 833
   2523 [N] WARNING: '!editorRectEvent.mSucceeded', file 
widget/ContentCache.cpp, line 256
   1156 [N] WARNING: NS_ENSURE_TRUE(selCon) failed: file 
editor/libeditor/nsEditor.cpp, line 631
   1112 [N] WARNING: No inner window available!: file 
dom/base/nsGlobalWindow.cpp, line 10013
   1080 [N] WARNING: Failed to unlock the wakelock.: '!rv.Failed()', file 
dom/html/HTMLMediaElement.cpp, line 2354
   1049 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 
0x80004005: file extensions/spellcheck/src/mozInlineSpellChecker.cpp, line 1995
948 [N] WARNING: Failed to retarget HTML data delivery to the parser 
thread.: file parser/html/nsHtml5StreamParser.cpp, line 951
918 [N] WARNING: NS_ENSURE_TRUE(selection-RangeCount()) failed: file 
editor/libeditor/nsHTMLEditRules.cpp, line 320
892 [N] WARNING: Silently denied access to property |undefined|: Access 
to privileged JS object not permitted 
(@resource://gre/modules/commonjs/toolkit/loader.js - 
resource://gre/modules/devtools/server/actors/script.js:1149): file 
js/xpconnect/wrappers/XrayWrapper.cpp, line 208
824 [N] WARNING: attempt to modify an immutable nsStandardURL: file 
netwerk/base/nsStandardURL.cpp, line 1264
823 [N] WARNING: NS_ENSURE_TRUE(selcon) failed: file 
editor/libeditor/nsEditor.cpp, line 658
823 [N] WARNING: NS_ENSURE_SUCCESS(res, nullptr) failed with result 
0xC1F30001: file editor/libeditor/nsEditor.cpp, line 667
687 [N] WARNING: Trying to spellcheck, but checking seems to be 
disabled: 'mPendingSpellCheck', file 
extensions/spellcheck/src/mozInlineSpellChecker.cpp, line 907
684 [N] WARNING: NS_ENSURE_TRUE(aSelection-RangeCount()) failed: file 
editor/libeditor/nsEditor.cpp, line 3725
684 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 
0x80004005: file editor/libeditor/nsEditor.cpp, line 3704
683 [N] WARNING: NS_ENSURE_SUCCESS(res, res) failed with result 
0x80004005: file editor/libeditor/nsHTMLEditRules.cpp, line 8083
675 [N] WARNING: Re-registering a CID?: file 
xpcom/components/nsComponentManager.cpp, line 551
655 [N] WARNING: NS_ENSURE_SUCCESS(rv, BadImage(newImage)) failed with 
result 0x80004005: file image/ImageFactory.cpp, line 248
653 [N] WARNING: NS_ENSURE_TRUE(globalObject  
globalObject-GetGlobalJSObject()) failed: file dom/base/nsDocument.cpp, line 
8323
628 [N] WARNING: Suboptimal indexes for the SQL statement 
0x7f30821a34a0 (http://mzl.la/1FuID0j).: file 
storage/mozStoragePrivateHelpers.cpp, line 109
604 [N] WARNING: NS_ENSURE_SUCCESS(status, status) failed with result 
0x805303F4: file dom/security/nsCORSListenerProxy.cpp, line 542
600 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 
0x805E000F: file dom/xbl/nsXBLService.cpp, line 732
597 [N] WARNING: '!aCharRect.width', file 

Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++

[Eric Rahm]

On Tuesday, July 7, 2015 at 3:30:59 PM UTC-7, Birunthan Mohanathas wrote:
 On 7 July 2015 at 15:02, Mike Hommey m...@glandium.org wrote:
  On Tue, Jul 07, 2015 at 11:52:12PM +0300, smaug wrote:
  until the tools (and poiru) are run and make the code follow Mozilla 
  coding style.
 
  Assuming you're talking about clang-format, that doesn't take care
  about anything else than whitespaces.
 
 I have a Clang tool to add the 'a' prefix, but it can be easily
 modified to drop the prefix should we decide to do so.

I'm not a huge fan of the 'aFoo' style, but I am a huge fan of consistency. So 
if we want to change the style guide we should update our codebase, and I don't 
think we can reasonably do that automatically without introducing shadowing 
issues.

Additionally I don't spend 50% of my time reviewing, so I'd say my opinion here 
(meh to aFoo) is less important. It's not an undue burden for me to include an 
aPrefix and if we have static analysis to check for it that would make it even 
less of an issue.

re: refactoring, I suppose that could be an argument against the aPrefix, but 
then again various IDE's make this a bit easier (including Eclipse, which I 
believe roc is a user of).

-e
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Revisiting modelines in source files

[Eric Rahm]

 If someone commits to producing a suitable clang-format binary/config, I'll
 sign up for creating all the tooling.

It looks like Birunthan stated he'd be willing to work on the clang-format side 
(I'm also happy to help out) if we are committed to integrating it into our 
wrorkflow.

I found an older bug [1] that Anthony Jones filed to upstream some patches he 
was working on, although I think that attempt was abandoned. Perhaps we can 
revive that?

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=961541
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Revisiting modelines in source files

[Eric Rahm]

On Thursday, June 18, 2015 at 7:28:44 AM UTC-7, kgu...@mozilla.com wrote:
 1) Comments that exceed the 80-char limit get wrapped blindly, rather than 
 being rewrapped properly. This results in comment blocks that look like this:
 
 // This is a comment that was previously just over the eighty
 // character
 // limit but got rewrapped by clang-format just blindly
 // inserting
 // linebreaks willy-nilly and requires manual fixup.
 
 I think this is an issue that should be fixed in clang-format. However, to 
 work around this incrementally I have been manually rewrapping comment blocks 
 in the APZ code if I'm touching them anyway.

Can we sidestep this by punting on enforcing a line length restriction with 
clang-format?

I think it would be reasonably uncontroversial to just do the following 
initially:
  - spacing
  - indentation
  - bracing

 2) MOZ_BEGIN_NESTED_ENUM_CLASS seems to trip up the formatter and it 
 misformats the code after that.

AFAICT there's no such thing as MOZ_BEGIN_NESTED_ENUM_CLASS

-e

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: The War on Warnings

[Eric Rahm]

On Wednesday, June 17, 2015 at 6:49:00 AM UTC-7, kgu...@mozilla.com wrote:
  4606 [N] WARNING: No widget found in TabParent::UpdateDimensions: file 
  dom/ipc/TabParent.cpp, line 974 
 
 Do you know if this one occurs on b2g or also on other platforms? I added 
 this warning recently in bug 1125325 after smaug said [1]. It seems to be 
 happening a lot, so we should investigate. If you have a bug on file for it 
 please cc me.
 
 [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1125325#c134

These are all within mochitest-e10s-browser-chrome tests, which AFAICT are not 
run on B2G, OSX, Windows debug builds. Filed bug 1175631 for the verbose 
warning.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: The War on Warnings

[Eric Rahm]

On Wednesday, June 17, 2015 at 7:40:14 AM UTC-7, Milan Sreckovic wrote:
 Do we have a all encompassing meta bug to collect all of the bugs that fix 
 the warnings?
 --
 - Milan
 

Bug 765224
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: The War on Warnings

[Eric Rahm]

An update on progress. I've landed bugs which should clean up ~180,000 
warnings. I have bugs pending for another ~26,000.

The latest top 40:

*Note: I improved my normalization a bit so it might look a bit different

18012 [N] WARNING: Subdocument container has no frame: file 
layout/base/nsDocumentViewer.cpp, line 2520
14816 [N] WARNING: NS_ENSURE_SUCCESS(EnsureScriptEnvironment(), nullptr) 
failed with result 0x: file docshell/base/nsDocShell.cpp, line 4605
9809 [N] WARNING: No docshells for remote frames!: file 
dom/base/nsFrameLoader.cpp, line 459
8929 [N] WARNING: Someone passed native anonymous content directly into 
frame construction.  Stop doing that!: file 
layout/base/nsCSSFrameConstructor.cpp, line 6559
8062 [N] WARNING: Suboptimal indexes for the SQL statement 0x 
(http://mzl.la/1FuID0j).: file storage/mozStoragePrivateHelpers.cpp, line 109
7760 [N] WARNING: NS_ENSURE_TRUE(mDocShell) failed: file 
embedding/browser/nsWebBrowser.cpp, line 363
7454 [N] WARNING: anonymous nodes should not be in child lists (bug 
439258): file layout/base/RestyleManager.cpp, line 1439
6294 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x: 
file netwerk/base/nsFileStreams.cpp, line 492
6294 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x: 
file netwerk/base/nsFileStreams.cpp, line 205
6131 [N] WARNING: No outer window available!: file 
dom/base/nsGlobalWindow.cpp, line 3920
5207 [N] WARNING: NS_ENSURE_TRUE(domWindow) failed: file 
embedding/browser/nsDocShellTreeOwner.cpp, line 83
5193 [N] WARNING: NS_ENSURE_TRUE(aInBrowser) failed: file 
embedding/browser/nsDocShellTreeOwner.cpp, line 79
5073 [N] WARNING: zero axis length: file dom/svg/nsSVGLength2.cpp, line 124
4606 [N] WARNING: No widget found in TabParent::UpdateDimensions: file 
dom/ipc/TabParent.cpp, line 974
4574 [N] WARNING: Shouldn't call SchedulePaint in a detached pres context: 
file layout/generic/nsFrame.cpp, line 5181
3891 [N] WARNING: Please do not use mouseenter/leave events in chrome. They 
are slower than mouseover/out!: '!nsContentUtils::IsChromeDoc(d)', file 
dom/events/EventListenerManager.cpp, line 367
3746 [N] WARNING: Graph thread slowdown?: 'std::abs(framePosition - 
CurrentDriver()-StateComputedTime())  MillisecondsToMediaTime(5)', file 
dom/media/MediaStreamGraph.cpp, line 1193
3182 [N] WARNING: Enabling vsync compositor: file 
gfx/layers/ipc/CompositorParent.cpp, line 674
3042 [N] WARNING: NS_ENSURE_TRUE(mCallback) failed: file 
dom/base/nsFrameMessageManager.cpp, line 805
2859 [N] WARNING: '!mContentCache.CacheEditorRect(this, 
aIMENotification)', file widget/PuppetWidget.cpp, line 819
2859 [N] WARNING: '!editorRectEvent.mSucceeded', file 
widget/ContentCache.cpp, line 499
2690 [N] WARNING: nsWindow::GetNativeData not implemented for this type: 
file widget/PuppetWidget.cpp, line 1058
2527 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x: 
file dom/base/nsContentUtils.cpp, line 3712
2526 [N] WARNING: NS_ENSURE_TRUE(domDoc  target) failed: file 
dom/base/nsContentUtils.cpp, line 3657
2478 [N] WARNING: Subdocument container has non-subdocument frame: file 
layout/base/nsDocumentViewer.cpp, line 2517
2064 [N] WARNING: NS_ENSURE_TRUE(newRoot) failed: file 
dom/base/nsRange.cpp, line 1132
2038 [N] WARNING: NS_ENSURE_TRUE(newRoot) failed: file 
dom/base/nsRange.cpp, line 1231
1912 [N] WARNING: NS_ENSURE_TRUE(rootContent) failed: file 
editor/composer/nsEditorSpellCheck.cpp, line 715
1627 [N] WARNING: Called close() before start()!: 'mStarted', file 
dom/workers/MessagePort.cpp, line 215
1612 [N] WARNING: NS_ENSURE_TRUE(sf) failed: file 
docshell/base/nsDocShell.cpp, line 6485
1488 [N] WARNING: NS_ENSURE_TRUE(isFileURI) failed: file 
dom/base/ThirdPartyUtil.cpp, line 368
1417 [N] WARNING: NS_ENSURE_SUCCESS(rv, false) failed with result 
0x: file netwerk/protocol/http/HttpBaseChannel.cpp, line 2077
1417 [N] WARNING: NS_ENSURE_SUCCESS(result, result) failed with result 
0x: file docshell/base/nsDocShell.cpp, line 14076
1393 [N] WARNING: Break suggested inside cluster!: file 
gfx/thebes/gfxTextRun.cpp, line 220
1288 [N] WARNING: NS_ENSURE_TRUE(mDisabledJSAndPlugins) failed: file 
editor/composer/nsEditingSession.cpp, line 209
1175 [N] WARNING: NS_ENSURE_TRUE(txToRemove) failed: file 
docshell/shistory/nsSHistory.cpp, line 1318
1151 [N] WARNING: NS_ENSURE_TRUE(selCon) failed: file 
editor/libeditor/nsEditor.cpp, line 631
1149 [N] WARNING: RemoveObserver() called for unregistered observer: file 
hal/Hal.cpp, line 205
1142 [N] WARNING: Failed to unlock the wakelock.: '!rv.Failed()', file 
dom/html/HTMLMediaElement.cpp, line 2341
1113 [N] WARNING: NS_ENSURE_TRUE(aURI) failed: file 
netwerk/dns/nsEffectiveTLDService.cpp, line 158

I have patches pending for #1 (bug 1175289) and #5 (bug 1175249) which 

Re: The War on Warnings

[Eric Rahm]

On Thursday, June 4, 2015 at 11:14:59 AM UTC-7, Martin Thomson wrote:
 On Jun 4, 2015 10:27 AM, Daniel Holbert dholb...@mozilla.com wrote:
  Also: in layout, there are various warnings related to coordinate
  wraparound/overflow, where we're basically throwing up our hands and
  warning that broken layout is likely to occur because the page is
  millions of pixels tall.  These can be useful hints, when debugging page
  brokenness.
 
 I know that it's more work, but isn't that what the console is best suited
 for? We have some spammy warnings there too, of course (try looking for rc4
 warnings if you have gmail open).

This does seem more appropriate for warnings targeted at web developers. Most 
end users probably will not be using debug builds of Firefox (particularly 
considering we market a dev edition, you know, for web devs).
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: The War on Warnings

[Eric Rahm]

On Thursday, June 4, 2015 at 1:48:30 PM UTC-7, Luke Wagner wrote:
 In addition to judging noisiness by volume over a whole test run, can we
 also include any warning that happens on normal browser startup, new tab,
 and other vanilla browser operations?  This has always annoyed me.

Yes, this bothers me as well. Here's a rough look at a run I just did (this is 
simply opening the browser to a blank page and then closing it):

 27 [N] WARNING: '!mMainThread', file 
/home/erahm/dev/mozilla-central/xpcom/threads/nsThreadManager.cpp, line 299
 10 [N] WARNING: 'NS_FAILED(DebuggerOnGCRunnable::Enqueue(aRuntime, 
aDesc))', file 
/home/erahm/dev/mozilla-central/xpcom/base/CycleCollectedJSRuntime.cpp, line 743
  5 [N] WARNING: XPCOM objects created/destroyed from static ctor/dtor: 
file /home/erahm/dev/mozilla-central/xpcom/base/nsTraceRefcnt.cpp, line 148
  4 [N] WARNING: RemoveObserver() called for unregistered observer: 
file /home/erahm/dev/mozilla-central/hal/Hal.cpp, line 205
  3 [N] WARNING: NS_ENSURE_TRUE(mDocShell) failed: file 
/home/erahm/dev/mozilla-central/embedding/browser/nsWebBrowser.cpp, line 363
  3 [N] WARNING: NS_ENSURE_SUCCESS(EnsureScriptEnvironment(), nullptr) 
failed with result 0x80040111: file 
/home/erahm/dev/mozilla-central/docshell/base/nsDocShell.cpp, line 4592
  2 [N] WARNING: Subdocument container has no frame: file 
/home/erahm/dev/mozilla-central/layout/base/nsDocumentViewer.cpp, line 2506
  2 [N] WARNING: Re-registering a CID?: file 
/home/erahm/dev/mozilla-central/xpcom/components/nsComponentManager.cpp, line 
551
  2 [N] WARNING: 'NS_FAILED(rv)', file 
/home/erahm/dev/mozilla-central/dom/workers/ServiceWorkerManager.cpp, line 2529
  2 [N] WARNING: NS_ENSURE_TRUE(domWindow) failed: file 
/home/erahm/dev/mozilla-central/embedding/browser/nsDocShellTreeOwner.cpp, line 
83
  2 [N] WARNING: NS_ENSURE_TRUE(aInBrowser) failed: file 
/home/erahm/dev/mozilla-central/embedding/browser/nsDocShellTreeOwner.cpp, line 
79
  2 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 
0x80040111: file /home/erahm/dev/mozilla-central/dom/base/nsFrameLoader.cpp, 
line 267
  2 [N] WARNING: Enabling vsync refresh driver: file 
/home/erahm/dev/mozilla-central/layout/base/nsRefreshDriver.cpp, line 859
  2 [N] WARNING: '!BasePrincipal::IsCodebasePrincipal(aPrincipal)', 
file /home/erahm/dev/mozilla-central/dom/workers/ServiceWorkerManager.cpp, line 
2591
  1 [N] WARNING: nsWindow::GetNativeData not implemented for this type: 
file /home/erahm/dev/mozilla-central/widget/PuppetWidget.cpp, line 1162
  1 [N] WARNING: NS_ENSURE_TRUE(domDoc  target) failed: file 
/home/erahm/dev/mozilla-central/dom/base/nsContentUtils.cpp, line 3636
  1 [N] WARNING: NS_ENSURE_TRUE(currentInner) failed: file 
/home/erahm/dev/mozilla-central/dom/base/nsGlobalWindow.cpp, line 9004
  1 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 
0x80070057: file /home/erahm/dev/mozilla-central/dom/base/nsContentUtils.cpp, 
line 3691
  1 [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 
0x80004005: file 
/home/erahm/dev/mozilla-central/toolkit/xre/nsXREDirProvider.cpp, line 1374
  1 [N] WARNING: No docshells for remote frames!: file 
/home/erahm/dev/mozilla-central/dom/base/nsFrameLoader.cpp, line 491
  1 [N] WARNING: Hardware Vsync support not yet implemented. Falling 
back to software timers: file 
/home/erahm/dev/mozilla-central/gfx/thebes/gfxPlatform.cpp, line 2410
  1 [N] WARNING: Enabling vsync compositor: file 
/home/erahm/dev/mozilla-central/gfx/layers/ipc/CompositorParent.cpp, line 679
  1 [N] WARNING: '!editorRectEvent.mSucceeded', file 
/home/erahm/dev/mozilla-central/widget/PuppetWidget.cpp, line 800
  1 [N] WARNING: Could not get disk status from nsIDiskSpaceWatcher: 
file 
/home/erahm/dev/mozilla-central/uriloader/prefetch/nsOfflineCacheUpdateService.cpp,
 line 317
  1 [N] WARNING: Could not get disk information from DiskSpaceWatcher: 
file /home/erahm/dev/mozilla-central/dom/storage/DOMStorageIPC.cpp, line 320
  1 [N] WARNING: '!compMgr', file 
/home/erahm/dev/mozilla-central/xpcom/glue/nsComponentManagerUtils.cpp, line 63

There's bug 1171716 for the !mMainThread warning.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

The War on Warnings

[Eric Rahm]

We emit a *lot* of runtime warnings when running debug tests. I inadvertently 
triggered a max log size failure during a landing this week which encouraged me 
to take a look at what all is being logged, and what I found was a ton of 
warnings (sometimes accompanied by stack traces). Most of these should probably 
be removed (of course if they're real issues they should be fixed, but judging 
by the frequency most are probably non-issues).

I'm currently cleaning up some of these, but if you happen to see something in 
the following list and are feeling proactive I would appreciate the help. 
There's even a meta bug for tracking these: 
https://bugzilla.mozilla.org/show_bug.cgi?id=765224

I generated this list by grabbing the logs for a recent m-c linux64 debug run, 
normalizing out PIDs and timestamps and then doing some sort/uniq-fu to get 
counts of unique lines.

This is roughly the top 40 offenders:

  65959   [N] WARNING: Overflowed nscoord_MAX in conversion to nscoord 
width: file ../../dist/include/nsRect.h, line 83
  63460   [N] WARNING: NS_ENSURE_TRUE(piTarget) failed: file 
gdom/events/EventDispatcher.cpp, line 469
  20039   [N] WARNING: 'NS_FAILED(rv)', file 
gdom/workers/ServiceWorkerManager.cpp, line 2529
  20039   [N] WARNING: '!BasePrincipal::IsCodebasePrincipal(aPrincipal)', 
file gdom/workers/ServiceWorkerManager.cpp, line 2591
  17784   [N] WARNING: Subdocument container has no frame: file 
glayout/base/nsDocumentViewer.cpp, line 2506
  16322   JavaScript warning: 
file:///builds/slave/test/build/tests/jsreftest/tests/js1_8/extensions/regress-476427.js,
 line 1: JavaScript 1.6's for-each-in loops are deprecated; consider using ES6 
for-of instead 
  14159   [N] WARNING: NS_ENSURE_TRUE(mMutable) failed: file 
gnetwerk/base/nsSimpleURI.cpp, line 264
  14087   [N] WARNING: NS_ENSURE_SUCCESS(EnsureScriptEnvironment(), 
nullptr) failed with result 0x80040111: file gdocshell/base/nsDocShell.cpp, 
line 4592
  11315   [N] WARNING: '!mMainThread', file 
gxpcom/threads/nsThreadManager.cpp, line 299 
  10574   [N] WARNING: No docshells for remote frames!: file 
gdom/base/nsFrameLoader.cpp, line 491
   9201   [N] WARNING: have unconstrained width; this should only result 
from very large sizes, not attempts at intrinsic width calculation: 'psd-mIEnd 
!= NS_UNCONSTRAINEDSIZE', file glayout/generic/nsLineLayout.cpp, line 884
   9155   [N] WARNING: have unconstrained width; this should only result 
from very large sizes, not attempts at intrinsic width calculation: 'psd-mIEnd 
!= NS_UNCONSTRAINEDSIZE', file glayout/generic/nsLineLayout.cpp, line 3058
   9130   [N] WARNING: have unconstrained width; this should only result 
from very large sizes, not attempts at intrinsic width calculation: 'aISize != 
NS_UNCONSTRAINEDSIZE', file glayout/generic/nsLineLayout.cpp, line 160
   8844   [N] WARNING: Someone passed native anonymous content directly 
into frame construction.  Stop doing that!: file 
glayout/base/nsCSSFrameConstructor.cpp, line 6559
   7599   [N] WARNING: NS_ENSURE_TRUE(mDocShell) failed: file 
gembedding/browser/nsWebBrowser.cpp, line 363
   7454   [N] WARNING: anonymous nodes should not be in child lists (bug 
439258): file glayout/base/RestyleManager.cpp, line 1440
   6544   [N] WARNING: Graph thread slowdown?: 'std::abs(framePosition - 
CurrentDriver()-StateComputedTime())  MillisecondsToMediaTime(5)', file 
gdom/media/MediaStreamGraph.cpp, line 1195
   6126   [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 
0x80520012: file gnetwerk/base/nsFileStreams.cpp, line 492
   6126   [N] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 
0x80520012: file gnetwerk/base/nsFileStreams.cpp, line 205
   5637   [N] WARNING: No outer window available!: file 
gdom/base/nsGlobalWindow.cpp, line 3915
   5109   [N] WARNING: NS_ENSURE_TRUE(domWindow) failed: file 
gembedding/browser/nsDocShellTreeOwner.cpp, line 83
   5085   [N] WARNING: NS_ENSURE_TRUE(aInBrowser) failed: file 
gembedding/browser/nsDocShellTreeOwner.cpp, line 79
   4856   [N] WARNING: zero axis length: file gdom/svg/nsSVGLength2.cpp, 
line 124
   4708   [N] WARNING: Shouldn't call SchedulePaint in a detached pres 
context: file glayout/generic/nsFrame.cpp, line 5181
   4051   [N] WARNING: have unconstrained inline-size; this should only 
result from very large sizes, not attempts at intrinsic inline-size 
calculation: '(mFrameType == NS_CSS_FRAME_TYPE_INLINE  
!frame-IsFrameOfType(nsIFrame::eReplaced)) || type == nsGkAtoms::textFrame || 
ComputedISize() != NS_UNCONSTRAINEDSIZE', file 
glayout/generic/nsHTMLReflowState.cpp, line 448
   4050   [N] WARNING: have unconstrained inline-size; this should only 
result from very large sizes, not attempts at intrinsic inline-size 
calculation: 'AvailableISize() != NS_UNCONSTRAINEDSIZE', file 
glayout/generic/nsHTMLReflowState.cpp, line 360
   3897   [N] WARNING: have unconstrained 

PSA: Goodbye PR_LOG, hello MOZ_LOG

[Eric Rahm]

tl;dr - PR_LOG* are deprecated. Use MOZ_LOG* instead.

As part of the effort to improving logging in gecko we've started a transition 
to using a MOZ prefix and a new set of log levels [1]. I've just landed these 
changes on mozilla-inbound, if they stick this means you should no longer use 
PR_LOG and friends (and in most cases it won't work as we explicitly undef them 
in mozilla/Logging.h).

A quick transition guide:
  - #include prlog.h = #include mozilla/Logging.h
  - PR_LOG = MOZ_LOG
  - PR_LOG_TEST = MOZ_LOG_TEST

Additionally we have moved to using an enum class to specify log levels. This 
means that using integer constants to represent log levels in code no longer 
works, nor can you create pseudo log levels (such as, lets say, PR_LOG_DEBUG + 
2).

After some rather thorough discussion [2] we have settled on an enum class that 
provides a pretty decent mapping to the previous PR_LOG levels:

enum class LogLevel {
  Disabled = 0,
  Error,
  Warning,
  Info,
  Debug,
  Verbose,
};

Of note:
  - PR_LOG_ALWAYS was removed, usage was mostly transitioned to the new Info 
log level. 
  - Verbose was added and instances of PR_LOG_DEBUG + 1 were replaced with it
  - PR_LOG_ERROR mapped to level 2, Error maps to level 1
  - PR_LOG_WARNING mapped to level 3, Warning maps to level 2

Additionally, please note, there is now an Info level. This should probably be 
the default output level for most messages, but, of course, which log level to 
use is left to the discretion of the developer.

Also as previously noted [3]: please stop using |#ifdef PR_LOGGING|, PR_LOGGING 
is always enabled. Such usage has no effect and is misleading.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1165515
[2] https://groups.google.com/forum/#!topic/mozilla.dev.platform/t9cDvC8LHsM
[3] https://groups.google.com/forum/#!topic/mozilla.dev.platform/JNN0T07y_ww
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Replacing PR_LOG levels

[Eric Rahm]

On Friday, May 22, 2015 at 12:06:05 PM UTC-7, David Rajchenbach-Teller wrote:
 On 22/05/15 04:06, Eric Rahm wrote:
  After a rather thorough examination of usages of each I have come up with a 
  set that I believe would fit our needs:
  
  enum class LogLevel {
Disabled = 0, // Logging is disabled for this module
Error,
Warning,
Info,
Debug,
  };
 
 I'll vote to have `Error` cause test failures unless explicitly whitelisted.
 
  
 
 
 -- 
 David Rajchenbach-Teller, PhD
  Performance Team, Mozilla

This is an interesting proposal. To make this work we'd need to:
  - Enable error level logging during test runs for all modules
  - Actually output the log level.
  - Do some automated analysis to figure out what needs to be whitelisted.

I'd suggest filing a bug to track this proposal (or perhaps split out a 
separate thread).

I think it could fit into the idea of structured logging as well, but as Nick 
pointed out that's some serious scope creep!
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Replacing PR_LOG levels

[Eric Rahm]

Thanks Adam, these are good points.

On Friday, May 22, 2015 at 2:08:33 PM UTC-7, Adam Roach wrote:
 On 5/22/15 15:51, Eric Rahm wrote:
  I agree, we shouldn't make it harder to turn on logging. The easy solution 
  is just to add a separate logger for verbose messages (if we choose not to 
  add Verbose/Trace).
 
 I don't know why we wouldn't just add a more verbose log level (Verbose, 
 Trace... I don't care what we call it). The presence of DEBUG + 1 in 
 our code is evidence of a clear, actual need.

This was my thought as well initially (my first proposal had a Verbose level). 
After auditing the code I changed my mind.

I believe adding the Info level will serve this need. The feeling I got from 
most code that did 'DEBUG + 1' was that they needed to use DEBUG for the 
informational stuff (because there was no Info level) and wanted a lower level 
for the more spammy stuff (DEBUG was occupied, so fine DEBUG + 1).

This was exemplified with the many variations of:
  #define LOGI(...) PR_LOG(..., PR_LOG_DEBUG)
  #define LOGV(...) PR_LOG(..., PR_LOG_DEBUG + 1)

Note: For LOGI, to me at least, 'I' = Info
Flipside of course the 'V' = Verbose, but I think this a semantic issue. Debug 
and Verbose are essentially the same thing now that we have Info.

We can see this ambiguity in various logging frameworks, generally there's a 
debug, but if there's not there's a verbose/trace, and usually not both.

 Making this a separate mechanism implies that the means of controlling 
 these more verbose messages is going to change at some point, and it 
 would be a change with no clear benefit. This means that, for example, 
 web pages intended to facilitate bug reporting [1] will need to be 
 updated to have variant instructions depending on the version of the 
 browser; and some such instructions are virtually guaranteed to be missed.

Regardless of what we do, we'll probably have to update our various docs. In 
fact the page you linked to is incorrect (which is part of why I'd like to move 
away from raw numbers). PR_LOG_DEBUG = 4, PR_LOG_DEBUG + 1 = 5. Setting log 
level 6 has no difference than 5 in this case.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Replacing PR_LOG levels

[Eric Rahm]

 
 +1 - I actually wasn't aware of this debug+1 mechanism and now that I am I
 would like to make use of it.

Please, please, please don't do this until we work this out (once we switch to 
enum class this will not be an option).

 requiring special builds is much much less satisfying, especially for
 resolving interop problems reported in bugzilla.

I agree, we shouldn't make it harder to turn on logging. The easy solution is 
just to add a separate logger for verbose messages (if we choose not to add 
Verbose/Trace).
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Replacing PR_LOG levels

[Eric Rahm]

Thank you for the feedback, you make some good points.

On Friday, May 22, 2015 at 1:31:14 AM UTC-7, masayuki nakano wrote:
 Hi,
 
 I still want a same level as PR_LOG_ALWAYS.

Would switching this to the new level 'Info' suffice? The main difference is 
that by enabling the Info level you will also see Error level messages as well. 

Currently it is the other way around, enabling Error level messages also 
enables Always level messages.

 In nsTextStore which is an implementation of ITextStoreACP of TSF uses 
 PR_LOG_ALWAYS only logs the module behavior. I.e., we can use to check 
 specific IME behavior with this level. I don't like to call this as 
 error. And also I wrote some documents how to log IME behavior. For 
 backward compatibility with such documents, I think that the info 
 should behave as PR_LOG_ALWAYS. SO, I think that the new enum should be:

I agree that calling those messages error is undesirable, my hope is that 
using info will be a good substitue.

Can we update these documents? When we add new ways to turn on logging at 
runtime we will need to update the

 enum class LogLevel
 {
Disabled = 0, // Logging is disabled for this module
Info,
Error,
Warning,
Debug,
 };

This diverges a fair amount from the established ordering of various other 
logging frameworks (see Ted's follow up, as well as mhoye's).
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Replacing PR_LOG levels

[Eric Rahm]

Thanks Randell, these are good points. I'll address a few of your comments that 
have not already been covered in the rest of the conversation.

 This is used extensively in WebRTC and related media bits to enable
 *huge* amounts of debugs (like every-frame debugs for audio or video or
 per-network-packet debugs, which will swamp a system normally), and since
 people are used to enabling debug on random modules (or all:5), having
 verbose debugs in the normal :5 setting will cause pain.
 

Here lies half the problem, that number does not mean what we were led to think 
it means. Using :5 would include PR_LOG_DEBUG + 1 [1].

 The above will also be surprising since it will work differently than
 other modules, making the same sorts of debugs appear at different
 levels.  This would be expecially confusing to people not frequently
 working in the module or when external users are asked to turn on
 debugging.

This assumes log levels have been used consistently across modules, I can 
assure you that is not the case :) I hope that we can get to a more consistent 
state of course!

I don't think it will be any more confusing when telling the external user what 
level to turn on (it's already rather confusing). Do we need super spammy? Turn 
on debug. Do we need just the basics? Turn on info.
 
 Then there's an oddball: webrtc.org Trace logging (a separate
 subsystem with low-overhead buffered-IO circular-log maxed at 10MB),
 which requires a 16-bit mask.  Currently this is exposed as
 webrtc_trace:65535 (for example), with a separate env var telling it
 where to put the logfile (or 'nspr' to revector the logs through NSPR
 logging, which *really* will cause problems with realtime code but is
 handy at times).  For this oddball, we could do other things like move
 it to a separate env var (which is annoying to people using it, but not
 horribly).

I think it's probably best to keep this a separate env rather than trying to 
shoehorn it in.

 what's the interface to NSPR_LOG_MODULES?  still with the numerics, or
 do we have to use
 NSPR_LOG_MODULES=signaling:debug,mediamanager:debug,getusermedia:debug,mtransport:debug,mediastreamgraph:debug
 etc?  (That gets a bit old... not that numerics are better, but
 they're faster to type/read and lots of existing scripts/etc have them.)
 Obviously one could have numbers and names.

Initially I plan to keep the same format for compatibility, eventually I would 
like to move away from it (or provide an improved method of setting log levels).

[1] 
https://hg.mozilla.org/mozilla-central/annotate/3e737d30f842/nsprpub/pr/include/prlog.h#l166
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Replacing PR_LOG levels

[Eric Rahm]

As part of the effort to improve logging in gecko we'd like to introduce a new 
set of unified log levels.

Currently we use NSPR logging which defines the following log levels:

typedef enum PRLogModuleLevel {
PR_LOG_NONE = 0,/* nothing */
PR_LOG_ALWAYS = 1,  /* always printed */  lies, damned lies
PR_LOG_ERROR = 2,   /* error messages */
PR_LOG_WARNING = 3, /* warning messages */
PR_LOG_DEBUG = 4,   /* debug messages */

PR_LOG_NOTICE = PR_LOG_DEBUG,   /* notice messages */
PR_LOG_WARN = PR_LOG_WARNING,   /* warning messages */
PR_LOG_MIN = PR_LOG_DEBUG,  /* minimal debugging messages */
PR_LOG_MAX = PR_LOG_DEBUG   /* maximal debugging messages */
} PRLogModuleLevel;

After a rather thorough examination of usages of each I have come up with a set 
that I believe would fit our needs:

enum class LogLevel {
  Disabled = 0, // Logging is disabled for this module
  Error,
  Warning,
  Info,
  Debug,
};

This subset maps rather nicely to syslog and web console logging levels:

| Level   | PR_LOG | Console   | Syslog  |
|-||---|-|
| Error   | PR_LOG_ERROR   | console.error()   | error   |
| Warning | PR_LOG_WARNING | console.warn()| warning |
| Info| n/a| console.info()| info|
| Debug   | PR_LOG_DEBUG   | console.debug()*  | debug   |

This removes all of the various PR_LOG aliases as well as |PR_LOG_ALWAYS|. It 
adds a generic |Info| level.

*PR_LOG_ALWAYS*

This really didn't mean what most uses thought it meant. Specifying 
|PR_LOG_ALWAYS| did not in fact mean your message was always logged, it just 
mean it was logged if at least log level 1 was specified. By default every 
logger has output disabled.

For most instances it can be replaced with |Info|, a more generic log category 
that is slightly more important than |Debug|, but not an error or warning.

*PR_LOG_DEBUG + 1 aka log level 5*

Various bits of code invented a log level that was less important than debug (I 
would call this verbose). This was not specified in NSPR logging, but just kind 
of worked. With the addition of |Info| we can transition code that was using 
this pseudo log level to:
  - map PR_LOG_DEBUG = Info
  - map PR_LOG_DEBUG + 1 = Debug

In this case we could have added a |Verbose| log level, but with the addition 
of |Info| and feeling that fewer is better we have decided against that.

*How will log levels be controlled?*

Similar to how it works now (if not terribly clear), turning on:
  - LogLevel::Error   = Error messages will be printed
  - LogLevel::Warning = Error and Warning messages will be printed
  - LogLevel::Info= Error, Warning, and Info messages will be printed
  - LogLevel::Debug   = Error, Warning, Info, and Debug messages will be 
printed

As a future improvement we plan to add the ability to toggle this levels at 
runtime.

Please let me know of any concerns or input you may have.

-e
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Meet mozilla/Logging.h

[Eric Rahm]

I just landed a patch [1] that replaces all instances of prlog.h in gecko 
code with mozilla/Logging.h. Currently it's not terribly interesting, it just 
includes prlog.h, but in the future we're going to start swapping out 
functionality.

To support this effort please use mozilla/Logging.h instead of prlog.h in 
gecko code.

If you need to fix up a patch probably all that's required is:
|sed -i -E -e 's/#include\s+(|)prlog.h(|)/#include 
mozilla\/Logging.h/'|

Other improvements pending [2]:
  - PR_LOG = MOZ_LOG
  - PR_LOG_TEST = MOZ_LOG_TEST
  - Remove usage of PR_LOG_ALWAYS
  - Remove usage of fake log levels (PR_LOG_DEBUG + 1, etc)
  - A fancy new LogLevel enum class

For the last 3 points I will follow up in separate thread explaining the 
current plan.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1165518
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1165515
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: PR_LOGGING is dead, please stop using it

[Eric Rahm]

Way back yonder in September we enabled logging by default on all builds. The 
world did not end. The |--disable-logging| flag remained which in theory, does 
in fact disable logging but none of our default builds use it, and 
unsurprisingly we occasionally started breaking custom builds that attempted to 
use it.

In bug 1161238 [1] we plan to remove |--disable-logging| completely, as part of 
that effort we have removed (or are about to) every instance of |#ifdef 
PR_LOGGING| from gecko code. So no more chance of breaking builds. But also now 
it's pretty clear if you are doing things you shouldn't. I found plenty of 
examples where it was assumed PR_LOGGING being defined was a rare thing (which, 
fair enough, it used to be). I've cleaned up most of the ones I found, but if 
you're responsible for code that is using logging I suggest you did a quick 
audit once I land bug 1161238.

They often took the form of something like this:

#ifdef PR_LOGGING
   nsCString message = do_a_couple_of_queries_and_build_a_msg();
   PR_LOG(foo, PR_LOG_DEBUG, (message = %s, message.get()));
#endif

This seems innocuous enough, it's protected by an ifdef! Unfortunately that 
ifdef is generally always true now. Fear not, there's a better way!

Wrap that sucker in a |PR_LOG_TEST|:

  if (PR_LOG_TEST(foo, PR_LOG_DEBUG)) {
nsCString message = do_a_couple_of_queries_and_build_a_msg();
PR_LOG(foo, PR_LOG_DEBUG, (message = %s, message.get()));
  }

Still sketched out? Is it debug only worthy? Well lets up that to a |#ifdef 
MOZ_DEBUG|:

#ifdef MOZ_DEBUG
  if (PR_LOG_TEST(foo, PR_LOG_DEBUG)) {
nsCString message = do_a_couple_of_queries_and_build_a_msg();
PR_LOG(foo, PR_LOG_DEBUG, (message = %s, message.get()));
  }
#endif

Note, something like the following is fine, it only gets evaluated if the log 
level is PR_LOG_DEBUG:

  PR_LOG(foo, PR_LOG_DEBUG, (message = %s, 
do_a_couple_of_queries_and_build_a_msg().get()));

In the coming months I hope to land further logging enhancements, stay tuned!

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1161238
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: JavaScript, const, and you

[Eric Rahm]

Just a heads up, a few of us have run into problems with extensions 
breaking in nightly. The problem is probably not restricted to extensions.


The scoping rules for const in JavaScript have changed to conform w/ the 
ES6 spec [1]. My understanding is that a const variable is now 
restricted to the scope it was declared in.


Basically the following no longer works:

  if (foo) {
 const bar = ...;
  }

  bar.baz();

This broke lightbeam [2], mozmill 1.5 (I believe it's fixed in the 2.0 
branch) which AWSY uses, and probably plenty of other things that have 
been using const since, I'm not sure when it showed up, but at least the 
past 4 years.


So if you're seeing failures since around 11/5 this could be it, you 
just need to rework the scope of your const statements (and maybe not 
use const unfortunately).


-e

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=611388
[2] https://github.com/mozilla/lightbeam/issues/622
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Enabling NSPR logging in release builds

[Eric Rahm]

On 10/3/14 1:12 PM, Eric Rahm wrote:

Hi all-

In bug 806819 we're planning on landing a change that allows us to turn
on NSPR logging in release builds [1]. To be clear, by default all
logging output is disabled, this will just allow you to turn on logging
via the same mechanisms [2] that have been available to debug builds and
modules that already force logging to be enabled.

Initial tests show no impact to performance, but of course it's possible
there will be unforeseen regressions. Testing included all Talos tests,
averaging of mochitest runtimes, and local ad-hoc performance measurements.

Areas to look out for:
   * Logging being done in hot areas. PR_LOG is no longer a no-op so
 there is a slight amount of overhead with each logging call. If
 your output is truly debug only consider wrapping in a #ifdef DEBUG
 block.
   * Creating a log message and then logging it. PR_LOG supports printf
 style formatting, so you can save some overhead by using that
 rather than writing to your own buffer.

 For example, if you once had:

   #ifdef PR_LOGGING
 char* msg = expensiveFunction();
 PR_LOG(module, PR_LOG_DEBUG, (msg));
   #endif

 You'll want to move to:

   PR_LOG(module, PR_LOG_DEBUG, (%s, expensiveFunction()));

If you're interested in making logging better, please take a look at our
meta bug [3] tracking various improvements. There's talk of making
improvements to NSPR logging, ditching NSPR logging all together, adding
streaming interfaces, switching log levels at runtime, and more. Feel
free to join in the conversation.

Please note: after this change you should never need to use FORCE_PR_LOG
(and you'll probably get build errors if you do).

A few side benefits:
   * All usage of FORCE_PR_LOG has been removed
   * Many more files are now eligible for unified building

-e

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=806819
[2]
https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSPR/Reference/Logging

[3] https://bugzilla.mozilla.org/show_bug.cgi?id=881389


This has landed, so far one issue has been reported for VS2013 builds 
[1] that has a pending solution for the underlying problem, this is not 
related to enabling logging but was exposed as part of the effort to get 
files added back in to unified compilation.


Many files were added back into unified compilation that were explicitly 
excluded due to NSPR logging and uses FORCE_PR_LOG (those are the 
terms I searched for), if you know of others that may not have been as 
easily found please follow up and add them back into the UNIFIED_SOURCES.


Also of course, don't use FORCE_PR_LOG anymore. You'll most likely get 
-Werror failures due to redefined macros. Please check your pending patches!


-e

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1080297
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Oculus VR support somehwat-non-free code in the tree

[Eric Rahm]

So who actually needs to talk to Oculus? I can try to reach out some 
folks I used to work with who are there now and see if they're 
interested in making license modifications.


-e

On 4/16/14, 7:01 AM, Gervase Markham wrote:

On 14/04/14 23:41, Vladimir Vukicevic wrote:

I'd like to get this checked in so that we can either have it enabled
by default in nightlies (and nightlies only), or at least allow it
enabled via a pref.  However, there's one issue -- the LibOVR library
has a not-fully-free-software license [1].  It's compatible with our
licenses, but it is not fully free.


As others in this thread have guessed, this discussion is the follow-up
to the bug Vlad opened to ask about this issue. It's here:
https://bugzilla.mozilla.org/show_bug.cgi?id=989903
although because it's a legal bug, sadly few of you will be able to see
it. I don't think there's anything secret in it, but it's not within my
power to open.

So people may be interested in what I said. My apologies that I am late
to this thread.

tl;dr: Henri is right that checking it in would reduce Oculus' desire to
work with us. We should talk to them first, and quickly.


1. Check in the LibOVR sources as-is, in other-licenses/oculus.  Add
a configure flag, maybe --disable-non-free, that disables building
it.  Build and ship it as normal in our builds.


The license concerned is here:
https://developer.oculusvr.com/license

Here are the issues I found with it after a quick read:

* You can only distribute or re-distribute LibOVR in whole, not in part.
An Open Source license must allow modifications and derived works (OSD).

* If your applications cause health and safety issues, or if you upset
them in other ways, you may lose your right to use the Oculus VR SDK,
including LibOVR. Open Source licenses need to be irrevocable, and they
need to not restrict what you can use the software for.

* Modifications to the Oculus VR SDK in source or binary form must be
shared with Oculus VR. The most that an Open Source license can require
is that they be shared with anyone to whom you give a copy of the
binary. This requirement goes beyond that.

* Certain sorts of changes have to be copyright-assigned to Oculus VR,
and you do not get a permissive license back to use them how you choose.

* You can't use the code to interface with other headsets. An Open
Source license cannot restrict what you can use the code for.

* They can change the license on you at any time. Open Source licenses
must not be revocable.

This license is a fairly long way from open source. So Mozilla policy
http://www.mozilla.org/MPL/license-policy.html would say that we don't
check this code into our repos (either in source or binary form) and
also, whether it's in our repos or not, we don't ship it with Firefox.

Making Firefox not-open-source would have a number of fairly serious
ramifications. Calling this licensing dogma is like calling the right
to trial-by-jury freedom dogma. As others have said, there are a large
number of people in the project to whom this is important.


2. Contact Oculus with our concerns about the license, and see if
they would be willing to relicense to something more standard.


I think we should definitely do this.


3. Start investigating Open VR, with the intent being to replace
the Oculus-specific library with a more standard one before we
standardize and ship the API more broadly than to nightly users.

The goal would be to remove LibOVR before we ship (or keep it in
assuming it gets relicensed, if appropriate), and replace it with a
standard Open VR library.


This strategy of using the Oculus code temporarily was not in view in
the original bug filed on this issue. It is perhaps an improvement, but
(without attributing bad motives to anyone) I suspect that once code is
in, integrated and working, the pressure to ship it would become so
great that the and replace it with some open source stuff later part
would get dropped and lost.

I can certainly name one Mozilla project where a non-open-source library
was used, and the bug to replace it with something open source never got
any attention after the code was shipped and working.


1. We could ship the VR glue in nightly, but the Oculus support
packaged as an addon.  This is doable, but it requires significant
rework in changing the interfaces to use XPCOM, to do category-based
registration of the Oculus provider, in building and packaging the
addon, etc.  It also requires a separate install step for
developers/users.


This is my proposal. Given that the hardware here costs $1500 and is
available in limited quantities, I am not too worried about the burden
on developers and users of installing an add-on.

Add-ons are where non-free code lives in the Firefox ecosystem. (Well,
and plugins, but we don't like them any more.)


Any objections to the above, or alternative suggestions?  This is a
departure in our current license policy, but not a huge one.


I think making Firefox non-free in the name