Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests

[David Major]

I agree that it's a bad idea for users to be running permanently with this
setting on their daily driver browsers.

But the environment variable has been a huge productivity enhancer to
reduce my mental load when setting up an extra-hairy debug session or
taking system traces.

I wish we could have a way to allow this for one-off cases but not
long-term usage. Unfortunately I can't settle for a proposal like "allow it
only in debug or only in nightlies" because I often need to debug actual
user-facing builds. Is there any way we could build some auto-expiration
into this setting, like maybe you'd have to set the env var equal to the
build ID or today's date?



On Wed, Jun 10, 2020 at 2:44 PM Dave Townsend  wrote:

> Non-e10s is such a different environment that I don't think we have any
> hope of keeping it working without running the full test suite in that mode
> and I don't think anyone wants to do that. Now that this has started
> breaking I think it is actively harmful to our users for us to allow them
> to disable e10s.
>
> On Wed, Jun 10, 2020 at 11:30 AM Gijs Kruitbosch  >
> wrote:
>
> > (Copied to fx-dev; Replies to dev-platform please.)
> >
> > Hello,
> >
> > Just over a year ago, I started a discussion[0] about our support for
> > disabling e10s. The outcome of that was that we removed support for
> > disabling e10s with a pref on Desktop Firefox with version 68, except
> > for use from automation. We kept support for using the environment
> > variable. [1]
> >
> > Last week, we released Firefox 77, which turned out to break all
> > webpages sent using compression (like gzip) if you had disabled e10s
> > using this environment variable. [2]
> >
> > So here we are again. I'd like to propose we also stop honouring the
> > environment variable unless we're running tests in automation. We
> > clearly do not have sufficient test coverage to guarantee basic things
> > like "the browser works", it lacks security sandboxing, and a number of
> > other projects require it (fission, gpu process, socket process, ...),
> > so I think it's time to stop supporting this configuration at all.
> >
> > I hope to make this change for the 79 cycle. I'm open to arguments
> > either way about what to do for 78 esr (assuming the patch for 79 turns
> > out to be simple; the work to remove the pref had a number of annoying
> > corner-cases at the time).
> >
> > Please speak up if you think that this plan needs adjusting.
> >
> > ~ Gijs
> >
> >
> > [0]
> >
> >
> https://groups.google.com/d/msg/mozilla.dev.platform/cJMzxi7_PmI/Pi1IOg_wCQAJ
> > [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1548941
> > [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1638652
> > ___
> > 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
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Faster compilers available in bootstrap

[David Major]

Yes, I believe so.

On Thu, Apr 9, 2020 at 1:09 PM Botond Ballo  wrote:

> Thanks, compile time improvements are always good news!
>
> Out of curiosity, does this impact builds targeting Android on Linux or
> Windows?
>
> Thanks,
> Botond
>
> On Thu, Apr 9, 2020 at 12:52 PM David Major  wrote:
> >
> > As of bug 1326486, our clang toolchains for Linux and Windows are built
> > with PGO. (Apologies to Mac users: that toolchain is cross-compiled.)
> >
> > Under optimal conditions (Spidermonkey build, touch mfbt) I've seen
> 10-15%
> > compile time improvements locally, but in more common scenarios the gains
> > will be diluted by other parts of the system. Perfherder measured 5-9%
> > improvements in CI.
> >
> > To pick up a new compiler: after the next time you update your tree, run
> > `./mach bootstrap` or `cd ~/.mozbuild && /path/to/mach artifact toolchain
> > --from-build linux64-clang`, or on Windows replace the last part with
> > `win64-clang-cl`.
> >
> > Happy compiling!
> > ___
> > 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

Faster compilers available in bootstrap

[David Major]

As of bug 1326486, our clang toolchains for Linux and Windows are built
with PGO. (Apologies to Mac users: that toolchain is cross-compiled.)

Under optimal conditions (Spidermonkey build, touch mfbt) I've seen 10-15%
compile time improvements locally, but in more common scenarios the gains
will be diluted by other parts of the system. Perfherder measured 5-9%
improvements in CI.

To pick up a new compiler: after the next time you update your tree, run
`./mach bootstrap` or `cd ~/.mozbuild && /path/to/mach artifact toolchain
--from-build linux64-clang`, or on Windows replace the last part with
`win64-clang-cl`.

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

Re: Lack of browser mochitests in non-e10s configuration and support for turning off e10s on desktop going forward

[David Major]

On Wed, Apr 24, 2019 at 1:39 PM Bobby Holley  wrote:
>
> Thanks Mike!
>
> So Fennec is the last remaining non-e10s configuration we ship to users.
> Given that Fennec test coverage is somewhat incomplete, we probably want to
> keep running desktop 1proc tests until Fennec EOL. We don't strictly need
> the browser-chrome tests, but we should probably avoid intentionally
> breaking non-e10s browser-chrome as long as engineers may still need to
> debug non-e10s test failures.
>
> After Fennec EOL, we basically have two options: a clean break where we rip
> out non-e10s support entirely, or a more muddled approach where we
> halfheartedly keep it working as a handy engineering hack as long as is
> practical. I think we should do the former.
>
> I don't want to downplay the handiness - being able to test and debug the
> browser in a single process can eliminate complexity and non-determinism,
> and save a lot of time. But at some point there's going to be a piece of
> core functionality that's too much work to keep supporting under non-e10s -
> and agonizing over that threshold on an ongoing basis is not a good use of
> anyone's time.

Why not have the conversation when such a piece of core functionality
arises? It's a much more convincing argument when you can say
"non-e10s needs to go away in order to support X".

In the meantime, single process debugging is a tremendous saver of
time and hassle, and we'd need a great reason to disable it. I'm not
convinced that one currently exists.

> Other opinions?
>
> On Tue, Apr 23, 2019 at 1:30 PM Mike Conley  wrote:
>
> > Hey folks,
> >
> > For Desktop, I don't believe there are any normal conditions under which
> > our users will have e10s disabled. [1] is where the decision gets made, and
> > it looks like these days, the only thing that will disable e10s is if the
> > user sets `browser.tabs.remote.autostart` to false themselves, or if a
> > MOZ_FORCE_DISABLE_E10S environment variable is set. I don't think either of
> > those are ever set by Mozilla these days.
> >
> > There was a case a few months back where e10s was disabled for users with
> > certain screen readers back for Firefox 60. Since that time, those screen
> > readers have updated to become more stable with e10s enabled.
> > Unfortunately, I don't have a reference to the bug(s) where that occurred,
> > but I spoke to yzen in #accessibility, and Firefox 60 ESR is the last
> > supported version where this e10s-disabling occurs on Desktop.
> >
> > So, to sum, I'm reasonably confident that, outside of Firefox 60 ESR,
> > e10s-disabled is not a mode that we ship to any of our users. We can
> > trigger it by pref flips or environment variables, but that's it.
> >
> > Mobile is another story - according to the fine folks in #mobile, Fennec
> > still runs Gecko in non-e10s mode.
> >
> > To circle back to Gijs's questions:
> >
> > 1. do we still consider running desktop Firefox with e10s disabled a
> >> supported configuration?
> >>
> >
> > Outside of Firefox 60 ESR, no, I don't believe so.
> >
> > 2. Will we need to turn it off on esr68 in the same circumstances where
> >> that happens on esr60?
> >>
> >
> > According to yzen from the Accessibility team, no, we won't.
> >
> > 3. If the answer to either of the previous 2 questions is 'yes', do we
> >> think it's acceptable not to run desktop tests on the configuration?
> >>
> >
> > Answers are no.
> >
> > 4. If the answer to both (1) and (2) is 'no', can we remove support for
> >> the pref and running desktop Firefox without e10s ? Some of the
> >> codepaths are not unified and so there is effectively dead code that
> >> we're lugging around for what would be no reason. (Note: a significant
> >> proportion isn't dead because even in e10s, we load some
> >> browser-provided content in parent process, ie a tab's browser is not
> >> always remote/non-same-process -- but even so, there's a bunch of stuff
> >> that keys off gMultiProcessBrowser that could be removed.)
> >>
> >
> > Yes, I believe that stuff is probably safe to remove at this point, as
> > long those changes don't assume e10s support on Fennec.
> >
> > -Mike
> >
> > [1]:
> > https://searchfox.org/mozilla-central/rev/ec489aa170b6486891cf3625717d6fa12bcd11c1/toolkit/xre/nsAppRunner.cpp#4964-5002
> >
> > On Tue, 23 Apr 2019 at 13:35, Dave Townsend  wrote:
> >
> >> Is there still a configuration (ignoring hidden prefs) that can cause a
> >> user to end up using non-e10s? If so we should turn these tests back on.
> >>
> >> On Tue, Apr 23, 2019 at 8:25 AM Joel Maher  wrote:
> >>
> >> > Here is where we initially turned on non-e10s tests for win7:
> >> > https://bugzilla.mozilla.org/show_bug.cgi?id=1391371
> >> >
> >> > and then moved to linux32:
> >> > https://bugzilla.mozilla.org/show_bug.cgi?id=1433276
> >> >
> >> > Currently mochitest-chrome and mochitest-a11y run as 1proc in-tree-
> >> these
> >> > run this way as they do not work with e10s=true.  I suspect the
> >> > mochitest-chrome is by design and 

Re: Improving our usage of Bugzilla

[David Major]

Will setting the "regressed by" field send mail to the subscribers of
the earlier bug? This was a useful aspect of the blocks/depends field.

On Tue, Mar 12, 2019 at 1:59 PM Sylvestre Ledru  wrote:
>
> Le 12/03/2019 à 17:48, Andrew McCreight a écrit :
> > On Tue, Mar 12, 2019 at 3:55 AM Sylvestre Ledru 
> > wrote:
> >
> >> With this change, we are going to extend Bugzilla to add a new field with
> >> three new values:
> >>
> >> -
> >>
> >> Defect - an issue in one of our products
> >> -
> >>
> >> Enhancement - a new feature or an improvement
> >> -
> >>
> >> Task - a developer task. For example: refactor code foo.
> >>
> >>
> > Could please you explain more what these three values are supposed to
> > represent, possibly with some examples? It feels like there's a lot of
> > overlap between them. For instance, isn't any task I work on as a developer
> > a developer task? Isn't refactoring both a task and an improvement (and
> > thus also an enhancement)?
>
> * Defects are trivial. Examples:
> Crashes, intermittent, glitches, features not working as expecting, features 
> worked
> but no longer works, etc.
>
> This is mostly for users of our products (including ourself). Triaged by eng 
> triage owners.
>
> We decided NOT to call that bug as this word is super-overloaded at Mozilla.
>
> * Enhancement is a development of a new feature.  Examples:
> Add a new avatar for sync, implement feature foo in Javascript,
> add bar property in css 42, etc
>
> This is mostly coming from users and us. Should be triagged by product and 
> triage owners.
>
> * Task is mostly development work. Examples:
> Refactor function foo, remove function bar, improve function plop, etc.
>
> Almost of them should be coming from engineering.
>
> Now, we will have cases for which a ticket will be both a task and an 
> enhancement.
> Example: https://bugzilla.mozilla.org/show_bug.cgi?id=1528330
> "Deliver acceptable GeckoView performance for Firefox Reality in H1"
> But I am sure you will agree that this new state will still be an improvement 
> over the current state.
>
>
> Now, a more concrete example, I have want to implement a new feature in 
> Firefox.
> I will create (or reuse) a bug with the "enhancement" value (probably as a 
> meta bug).
> Then, to develop the feature, I will create various "tasks" which will be 
> marking as
> blocking the meta bug.
>
> Because I am a bad developer, I will make mistakes and user will fill bugs 
> which
> will have the "defect" value (and I will use the regressed_by field).
>
>
> > Secondly, to bikeshed a little, could there be some name besides "task" for
> > that third category? Like I said above, everything we work as developers is
> > a developer task. "Refactor" seems like a clearer name, though maybe it is
> > a little limiting. "Side grade"? :)
>
> This is more than just refactoring. It is more "as an engineer, here is what 
> I have to do".
>
> About bikeshed, you can have a look here 
> https://bugzilla.mozilla.org/show_bug.cgi?id=1522342
> Where we did that.
>
> > Thirdly, what category should "organizational" bugs like meta bugs be
> > assigned to? I guess if you have a meta bug for a bunch of enhancements, it
> > would be an enhancement?
>
> Yeah, "it depends" ;) Probably...
>
>
> >
> > 3) Adding a new field called “Regressed by”
> > This is great! The weird encoding of "regressed by" in the "depends on"
> > field is one of the more confusing aspects of Bugzilla, given how important
> > it is. I spend a ton of time setting regressions for bugs, and even still I
> > have to stop occasionally to make sure I'm doing it the right way.
>
> Same here ;)
>
>
> S
>
>
> ___
> 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: Min clang / libclang requirement was updated not long ago...

[David Major]

https://bugzil.la/bootstrap-toolchain-redownloads (it's even got a
name!) is a really annoying one that's been on file for 6+ months.

On Wed, Feb 27, 2019 at 8:39 AM Nathan Froyd  wrote:
>
> On Wed, Feb 27, 2019 at 6:22 AM Kartikaya Gupta  wrote:
> > On Wed, Feb 27, 2019 at 3:40 AM Axel Hecht  wrote:
> > >
> > > Can we please not force bootstrap?
> >
> > +1. In general bootstrap isn't "rock solid" enough to force people
> > into running it.
>
> If people have problems with bootstrap (it doesn't do enough, it
> assumes too much about your system, etc. etc.), please file bugs on
> what's wrong.  We need to start depending more on bootstrap for
> everything, to the point of "you can't depend on X unless it gets
> installed via bootstrap", and we can't get to that world if we don't
> know what rough edges people find in bootstrap.
>
> Thanks,
> -Nathan
> ___
> 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: Min clang / libclang requirement was updated not long ago...

[David Major]

Does configure warn about this?

The link between this error and needing to bootstrap is not super
clear (and a surprising number of people don't read dev-platform) so
I'm not looking forward to answering the same question in #build for
the rest of the week. :)

On Tue, Feb 26, 2019 at 12:23 PM Emilio Cobos Álvarez  wrote:
>
> ... so if you don't use the mozilla-provided libclang (or are using a
> very old one), and you see an error like:
>
> > error[E0277]: the trait bound
> `values::generics::rect::Rect f32>>:
> std::convert::From,
>
> Please re-run mach bootstrap, or update your libclang.
>
> Thanks!
>
>  -- Emilio
> ___
> 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: Type-based alias analysis and Gecko C++

[David Major]

I don't think anyone wants to allow aliasing merely for its own sake.
A lot of these flags were added just to keep builds working in the
face of noisy compilers a long time ago. It would be good to retest
with our current codebase and current compilers and see where we
stand. If we can easily remove (or reduce) uses of this flag, I think
that would be pretty uncontroversial. But if it turns out to be a huge
amount of work, then as nice as it would be to do the cleanup, it
might not be a justifiable use of time and opportunity cost.


On Fri, Feb 15, 2019 at 3:59 AM Henri Sivonen  wrote:
>
> I happened to have a reason to run our build system under strace, and
> I noticed that we pass -fno-strict-aliasing to clang.
>
> How committed are we to -fno-strict-aliasing?
>
> If we have no intention of getting rid of -fno-strict-aliasing, it
> would make sense to document this at
> https://developer.mozilla.org/en-US/docs/Mozilla/Using_CXX_in_Mozilla_code
> and make it explicitly OK for Gecko developers not to worry about
> type-based alias analysis UB--just like we don't worry about writing
> exception-safe code.
>
> Debating in design/review or making an effort to avoid type-based
> alias analysis UB is not a good use of time if we're committed to not
> having type-based alias analysis.
>
> --
> Henri Sivonen
> hsivo...@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

Re: Moving reviews to Phabricator

[David Major]

For what it's worth, arcanist works fine for me in WSL, with a
considerably less-horrifying installation process than on real
Windows.

With an alias you can call it from MozillaBuild as if it were native:
alias arc="cmd //c ubuntu1804 run arc"


On Wed, Feb 6, 2019 at 3:49 PM Jörg Knobloch  wrote:
>
> On 06/02/2019 21:42, Kim Moir wrote:
> > On February 28 Bugzilla review flags will be disabled for Firefox and other
> > mozilla-central products and components. From this point forward, all
> > reviews of code changes to mozilla-central should be conducted in
> > Phabricator. Tasks that have been identified as crucial to this transition
> > have been set as blocker bugs tohttps://bugzil.la/1514775.
>
> Any simplification in sight to get this set up more easily for Windows
> users?
>
> Last I looked you needed 12 steps to get there:
> https://moz-conduit.readthedocs.io/en/latest/arcanist-windows.html
>
> I have three Windows development machines to set up :-(
>
> Jörg.
>
>
> ___
> 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

PSA: Inadvertently exporting third-party symbols

[David Major]

Hi,

As importing third-party code into libxul seems to be pretty popular,
I wanted to point out something that's easy to overlook.

Libraries usually have code that goes like:

#ifdef _WIN32
#define MYLIB_EXPORT __declspec(dllexport)
#else
#define MYLIB_EXPORT __attribute__((visibility("default")))
#endif

MYLIB_EXPORT int MyLibrary_DoTheThing() { return 42; }

This makes perfect sense when upstream builds their product as
MyLibrary.dll, as those APIs need to be callable from the outside
world. But when we paste the code into libxul, and its callers are
also in libxul, there is no need to cross a library boundary.

Exporting those symbols when not needed is harmful for several reasons:

* It prevents optimization and/or increases code size. The compiler
has to choose between keeping a single copy of the function with the
shape demanded by the ABI, which inhibits interprocedural
optimizations; or creating optimized/inlined copies but keeping the
original just in case the outside world ever calls it.

* It attracts wrongful blame in crash stacks. In cases where a
debugger doesn't have access to symbols, it will guess an
instruction's function based on the nearest exported symbol. This
often makes it look like function_that_landed_last_week+0x12345 caused
a crash, which wastes time with a wild goose chase.

* Exported symbols invite function hooks from questionable software.

I've filed bug 1523352 to see if we can keep track of export table
size in Perfherder similar to the existing section size metrics.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Pointer to the stack limit

[David Major]

You'll need platform-specific code, but on Windows there's 
https://searchfox.org/mozilla-central/rev/13788edbabb04d004e4a1ceff41d4de68a8320a2/js/xpconnect/src/XPCJSContext.cpp#986.

And, to get a sense of caution, have a look at the ifdef madness surrounding 
the caller -- 
https://searchfox.org/mozilla-central/rev/13788edbabb04d004e4a1ceff41d4de68a8320a2/js/xpconnect/src/XPCJSContext.cpp#1125
 -- to see the number of hoops we have to jump through to accommodate various 
build configs.

On Wed, Dec 19, 2018, at 7:12 AM, Henri Sivonen wrote:
> Is it possible to dynamically at run-time obtain a pointer to call
> stack limit? I mean the address that is the lowest address that the
> run-time stack can grow into without the process getting terminated
> with a stack overflow.
> 
> I'm particularly interested in a solution that'd work on 32-bit
> Windows and on Dalvik. (On ART, desktop Linux, and 64-bit platforms we
> can make the stack "large enough" anyway.)
> 
> Use case: Implementing a dynamic recursion limit.
> 
> -- 
> Henri Sivonen
> hsivo...@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

Re: Using clang-cl to ship Windows builds

[David Major]

A quick update about performance: ThinLTO and PGO were enabled for our
clang-cl builds over the last few weeks. These optimizations have
cancelled the previous regressions and brought clang well ahead of
MSVC: 
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try=2adb3cb404ad=try=5336df94a9ce=1
To call out a few: Speedometer scores increased by 6% on Win64 and 12%
on Win32, and the most dramatic improvement was displaylist_mutate at
27%!

As far as I'm aware, no blockers have come up and clang-cl is still
looking good for the 63 train.

On Tue, Jul 10, 2018 at 4:29 PM David Major  wrote:
>
> Bug 1443590 is switching our official Windows builds to use clang-cl
> as the compiler.
>
> Please keep an eye out for regressions and file a blocking bug for
> anything that might be fallout from this change. I'm especially
> interested in hearing about the quality of the debugging experience.
>
> It's possible that the patch may bounce and we'll go back and forth to
> MSVC for a while. You can check your build's compiler at
> `about:buildconfig`. Treeherder is running an additional set of MSVC
> jobs on mozilla-central to make sure we can fall back to a green MSVC
> if needed.
>
> Watch for more toolchain changes to come. The next steps after this
> will be to switch to lld-link and enable ThinLTO. That will open the
> door to a cross-language LTO that could inline calls between Rust and
> C++. In the longer term we can look into cross-compiling from Linux.
>
> But for now, shipping our most-used platform with an open-source
> compiler is a huge milestone in and of itself. Big thanks to everyone
> who has contributed to this effort on the Mozilla side, and also big
> thanks to the developers of LLVM and Chromium who helped make clang on
> Windows a realistic possibility.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Local Windows builds to use clang-cl by default

[David Major]

Hi,

Local Windows builds will use clang-cl and lld-link by default as of
bug 1483835. This will make your builds match what has been shipping
on Nightly for the last several weeks, and help avoid the occasional
MSVC bustage that sneaks in now that those builds are tier-2.

If you already ran `mach bootstrap` for the recent cbindgen changes,
you'll already have an up-to-date clang and there's no further action
required.

If you need to revert to the Microsoft toolchain, you can put one or
both of these lines in your mozconfig (but please get in touch and
tell me why you needed to):
export CC=cl
export LINKER=link

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

Re: Fission MemShrink Newsletter #1: What (it is) and Why (it matters to you)

[David Major]

This touches on a really important point: we're not the only ones
allocating memory.

Just a few that come to mind: GPU drivers, system media codecs, a11y
tools, and especially on Windows we have to deal with "utility"
applications, corporate-mandated gunk, and downright crapware.

When we're measuring progress toward our goals, look at not only your
own pristine dev box but also that one neighbor whose adware you're
always cleaning out.


On Fri, Jul 13, 2018 at 7:57 AM Gabriele Svelto  wrote:
>
> Just another bit of info to raise awareness on a thorny issue we have to
> face if we want to significantly raise the number of content processes.
> On 64-bit Windows we often consume significantly more commit space than
> physical memory. This consumption is currently unaccounted for in
> about:memory though I've seen hints of it being cause by the GPU driver
> (or other parts of the graphics pipeline). I've filed bug 1475518 [1] so
> that I don't forget and I encourage anybody with Windows experience to
> have a look because it's something we _need_ to solve to reduce content
> process memory usage.
>
>  Gabriele
>
> [1] Commit-space usage investigation
> https://bugzilla.mozilla.org/show_bug.cgi?id=1475518
>
> ___
> 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: Using clang-cl to ship Windows builds

[David Major]

At the moment, performance is a mixed bag. Some tests are up and some
are down. In particular I believe Speedometer is down a few percent.

Note however that clang-cl is punching above its weight. These builds
currently have neither LTO nor PGO, while our MSVC builds use both of
those. Any regressions that we're seeing ought to be short-lived. Once
we enable LTO and PGO, I expect clang to be a clear performance win.

If the short-term regressions end up being unacceptable, we can revert
to MSVC later in the release, but at this early point in the cycle
that shouldn't prevent us from collecting data on Nightly.

On Tue, Jul 10, 2018 at 4:31 PM Chris Peterson  wrote:
>
> How does the performance of clang-cl builds compare to MSVC builds on
> benchmarks like Speedometer?
>
>
> On 2018-07-10 1:29 PM, David Major wrote:
> > Bug 1443590 is switching our official Windows builds to use clang-cl
> > as the compiler.
> >
> > Please keep an eye out for regressions and file a blocking bug for
> > anything that might be fallout from this change. I'm especially
> > interested in hearing about the quality of the debugging experience.
> >
> > It's possible that the patch may bounce and we'll go back and forth to
> > MSVC for a while. You can check your build's compiler at
> > `about:buildconfig`. Treeherder is running an additional set of MSVC
> > jobs on mozilla-central to make sure we can fall back to a green MSVC
> > if needed.
> >
> > Watch for more toolchain changes to come. The next steps after this
> > will be to switch to lld-link and enable ThinLTO. That will open the
> > door to a cross-language LTO that could inline calls between Rust and
> > C++. In the longer term we can look into cross-compiling from Linux.
> >
> > But for now, shipping our most-used platform with an open-source
> > compiler is a huge milestone in and of itself. Big thanks to everyone
> > who has contributed to this effort on the Mozilla side, and also big
> > thanks to the developers of LLVM and Chromium who helped make clang on
> > Windows a realistic possibility.
> > ___
> > 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

Using clang-cl to ship Windows builds

[David Major]

Bug 1443590 is switching our official Windows builds to use clang-cl
as the compiler.

Please keep an eye out for regressions and file a blocking bug for
anything that might be fallout from this change. I'm especially
interested in hearing about the quality of the debugging experience.

It's possible that the patch may bounce and we'll go back and forth to
MSVC for a while. You can check your build's compiler at
`about:buildconfig`. Treeherder is running an additional set of MSVC
jobs on mozilla-central to make sure we can fall back to a green MSVC
if needed.

Watch for more toolchain changes to come. The next steps after this
will be to switch to lld-link and enable ThinLTO. That will open the
door to a cross-language LTO that could inline calls between Rust and
C++. In the longer term we can look into cross-compiling from Linux.

But for now, shipping our most-used platform with an open-source
compiler is a huge milestone in and of itself. Big thanks to everyone
who has contributed to this effort on the Mozilla side, and also big
thanks to the developers of LLVM and Chromium who helped make clang on
Windows a realistic possibility.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Windows Address Sanitizer enabled on trunk

[David Major]

As of bug 1467126 these jobs are now running at tier 1.

On Fri, Jun 1, 2018 at 3:16 PM David Major  wrote:
>
> Bug 1360120 on inbound enables Windows ASan builds and tests on trunk 
> branches.
>
> Initially these are tier-2 while we confirm that this doesn't
> introduce test flakiness. If nothing catches fire, I intend to bump
> them to tier-1 in the near future.
>
> You can run these jobs on try under the platform name "win64-asan" or
> build locally with the mozconfig here:
> https://developer.mozilla.org/en-US/docs/Mozilla/Testing/Firefox_and_Address_Sanitizer#Creating_local_builds_on_Windows
>
> If you're building ASan locally on Windows 10 version 1803, you'll
> want to run mach bootstrap to pick up a fresh clang with an
> 1803-specific fix.
>
> This platform has taken several years to stand up. Thank you to
> everyone who helped out, especially Ehsan for getting this started and
> Ting-Yu for working through a bunch of hurdles on automation.
>
> Happy sanitizing!
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Windows Address Sanitizer enabled on trunk

[David Major]

Bug 1360120 on inbound enables Windows ASan builds and tests on trunk branches.

Initially these are tier-2 while we confirm that this doesn't
introduce test flakiness. If nothing catches fire, I intend to bump
them to tier-1 in the near future.

You can run these jobs on try under the platform name "win64-asan" or
build locally with the mozconfig here:
https://developer.mozilla.org/en-US/docs/Mozilla/Testing/Firefox_and_Address_Sanitizer#Creating_local_builds_on_Windows

If you're building ASan locally on Windows 10 version 1803, you'll
want to run mach bootstrap to pick up a fresh clang with an
1803-specific fix.

This platform has taken several years to stand up. Thank you to
everyone who helped out, especially Ehsan for getting this started and
Ting-Yu for working through a bunch of hurdles on automation.

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

Re: Update on rustc/clang goodness

[David Major]

We've confirmed that this issue with debug symbols comes from lld-link and
not from clang-cl. This will likely need a fix from the LLVM side, but in
the meantime I'd like to encourage people not to be deterred from using
clang-cl as your compiler.

On Thu, May 10, 2018 at 9:12 PM Xidorn Quan  wrote:

> On Fri, May 11, 2018, at 10:35 AM, Anthony Jones wrote:
> > I have some specific requests for you:
> >
> > Let me know if you have specific Firefox related cases where Rust is
> > slowing you down (thanks Jeff [7])
> > Cross language inlining is coming - avoid duplication between Rust
> > and C++ in the name of performance
> > Do developer builds with clang

> Regarding the last item about building with clang on Windows, I'd not
recommend people who use Visual Studio for debugging Windows build to build
with clang at this moment.

> I've tried using lld-link as linker (while continuing using cl rather
than clang-cl) for my local Windows build, and it seems to cause problems
when debugging with Visual Studio. Specifically, you may not be able to
invoke debugging functions like DumpJSStack, DumpFrameTree in Immediate
Windows, and variable value watching doesn't seem to work well either.

> I've filed a bug[1] for the debugging function issue (and probably should
file another for the watching issue as well).

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


> - Xidorn
> ___
> 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: Firefox 60 Beta build error on ARM

[David Major]

This sounds like https://bugzilla.mozilla.org/show_bug.cgi?id=1434589
which currently doesn't have a fix. You might be able to work around
it for now with --disable-webrtc.

On Wed, May 2, 2018 at 1:08 PM, Charles G Robertson
 wrote:
> Hi,
>
> I'm trying to build Firefox 60 Beta on Arm64 and seeing this error:
> ...
> g++: error: unrecognized command line option '-msse2'
> gmake[4]: *** [/home/abuild/rpmbuild/BUILD/mozilla/config/rules.mk:1049: 
> Unified_cpp_common_audio_sse2_gn0.o] Error 1
> gmake[3]:
>   *** [/home/abuild/rpmbuild/BUILD/mozilla/config/recurse.mk:73:
> media/webrtc/trunk/webrtc/common_audio/common_audio_sse2_gn/target]
> Error 2
> gmake[3]: *** Waiting for unfinished jobs
> ...
>
> Had no
> issue building Firefox 59 for Arm64 so something changed between 59 and 60. 
> Is there some configure option I
> should be enabling/disabling when building FF 60 on Arm?
>
> Thanks,
> Charles Robertson
> SUSE
>
> ___
> 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: Please try out clang-cl and lld-link on Windows

[David Major]

The SDK requirement is due to some nonstandard code in the latest Microsoft
headers that clang rejects:
https://developercommunity.visualstudio.com/content/problem/132223/clang-cant-compile-wrlimplementsh.html

For now the easiest way forward is to go into Visual Studio Installer, add
"Windows 10 SDK (10.0.15063.0) for Desktop C++ [x86 and x64]" and remove
16299.

On Wed, Mar 14, 2018 at 6:20 AM, Marco Bonardo <mbona...@mozilla.com> wrote:

> It sounds cool and I'd really love to try, but:
> "ERROR: Found SDK version 10.0.16299.0 but clang-cl builds currently
> don't work with SDK version 10.0.16299.0 and later..."
>
> I know that I can set WINDOWSSDKDIR, but I'm not willing to mess too
> much with the env. Is there a bug tracking the update to the latest
> sdk, or automatically use the right one, that I can follow?
>
>
> On Tue, Mar 13, 2018 at 3:31 PM, David Major <dma...@mozilla.com> wrote:
> > Link xul.dll in 20 seconds with this one weird trick!
> >
> >
> >
> > Hi everyone,
> >
> >
> > clang-cl builds of Firefox have come a long way, from being a hobby
> project
> > of a few developers to running static analysis in CI for more than a year
> > now. The tools are in really good shape and should be ready for broader
> use
> > within Mozilla at this point.
> >
> >
> > Bug 1443590 is looking into what it would take to ship official builds
> with
> > clang-cl and lld-link, but in the meantime it's possible to do local
> builds
> > already. I'd like to invite people who develop on Windows to give it a
> try.
> >
> >
> > *** Reasons to use clang-cl and lld-link locally ***
> >
> >
> > - Speed! lld is known for being very fast. I'm serious about 20-second
> > libxuls. That's a non-incremental link, with identical code folding
> enabled.
> > For comparison, MSVC takes me over two minutes.
> >
> > - Speed again! clang-cl will integrate with upcoming sccache/icecream
> work.
> >
> > - Much clearer and more actionable error messages than MSVC
> >
> > - Make your own ASan and static analysis builds (the latter need an LLVM
> > before r318304, see bug 1427808)
> >
> > - Help ship Firefox with clang-cl by getting more eyes and machines on
> these
> > tools
> >
> >
> > *** Reasons not to use clang-cl and lld-link locally (yet) ***
> >
> >
> > - You are testing codegen-level fixes or optimizations and need to see
> the
> > exact bits that will be going out to users
> >
> > - lld-link currently doesn’t support incremental linking -- but with full
> > linking being so fast, this might not matter
> >
> > - You do artifact builds that don't use a local compiler
> >
> >
> > *** How do I get started? ***
> >
> >
> > https://developer.mozilla.org/en-US/docs/Mozilla/Developer_
> guide/Build_Instructions/Building_Firefox_on_Windows_with_clang-cl
> >
> >
> > A number of build system changes have landed that make these builds much
> > easier than before. For example you no longer need to use old versions of
> > MozillaBuild.
> >
> >
> > Note that clang-cl builds still depend on an MSVC installation for
> headers,
> > libraries, and auxiliary build tools, so don't go uninstalling your
> Visual
> > Studio just yet.
> >
> >
> > If you run into any problems, please stop by #build or visit the shiny
> new
> > Firefox Build System product in Bugzilla (formerly Core :: Build Config).
> >
> >
> > Thanks!
> >
> >
> >
> > ___
> > 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

Please try out clang-cl and lld-link on Windows

[David Major]

Link xul.dll in 20 seconds with this one weird trick!


Hi everyone,

clang-cl builds of Firefox have come a long way, from being a hobby project
of a few developers to running static analysis in CI for more than a year
now. The tools are in really good shape and should be ready for broader use
within Mozilla at this point.

Bug 1443590 is looking into what it would take to ship official builds with
clang-cl and lld-link, but in the meantime it's possible to do local builds
already. I'd like to invite people who develop on Windows to give it a try.

*** Reasons to use clang-cl and lld-link locally ***

- Speed! lld is known for being very fast. I'm serious about 20-second
libxuls. That's a non-incremental link, with identical code folding
enabled. For comparison, MSVC takes me over two minutes.

- Speed again! clang-cl will integrate with upcoming sccache/icecream work.

- Much clearer and more actionable error messages than MSVC

- Make your own ASan and static analysis builds (the latter need an LLVM
before r318304, see bug 1427808)

- Help ship Firefox with clang-cl by getting more eyes and machines on
these tools

*** Reasons not to use clang-cl and lld-link locally (yet) ***

- You are testing codegen-level fixes or optimizations and need to see the
exact bits that will be going out to users

- lld-link currently doesn’t support incremental linking -- but with full
linking being so fast, this might not matter

- You do artifact builds that don't use a local compiler

*** How do I get started? ***

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Building_Firefox_on_Windows_with_clang-cl

A number of build system changes have landed that make these builds much
easier than before. For example you no longer need to use old versions of
MozillaBuild.

Note that clang-cl builds still depend on an MSVC installation for headers,
libraries, and auxiliary build tools, so don't go uninstalling your Visual
Studio just yet.

If you run into any problems, please stop by #build or visit the shiny new
Firefox Build System product in Bugzilla (formerly Core :: Build Config).

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

Re: Pulsebot in #developers

[David Major]

I use #developers for two things:

1. I prefer to keep my discussions in smaller topic channels, but for my
sanity I also try to keep my channel list small. There is a large set of
people whom I ping roughly once a month and can't be bothered matching
channels with. #developers is my "lowest common denominator" for talking to
those people.

2. To cast the widest possible net for immediate help with some sudden
roadblock or unfamiliar tool etc.

I don't really care whether pulsebot gets moved, but it won't change my use
of #developers.


On Mon, Nov 6, 2017 at 8:13 AM, Gijs Kruitbosch 
wrote:

> On 06/11/2017 12:49, Philipp Kewisch wrote:
>
>> If there is a better place to ask ad-hoc questions about Gecko, I am
>> happy to go there (and we should make sure the channel is promoted in
>> our docs).
>>
>
> I think different teams have ended up with different IRC channels, as Kris
> said. Frontend Firefox stuff tends to go in #fx-team, and #content, #jsapi
> and similar channels each cater to their own "niche", so to speak.
>
> I can see how less usage of IRC can lead to the situation we have now.
>> As long as there is an (open) replacement that is more accepted nowadays
>> I think this is fine, but aside from that I think we should find a way
>> to promote discussing Firefox and Platform issues in the open again.
>>
>
> I am not aware of people discussing Firefox or Platform *engineering*
> issues in other places, open or otherwise. I am aware of incidental
> non-engineering folks using Slack for bugreporting (unsurprisingly, a lot
> of those questions get answered with "file a bug in bugzilla"), but it
> seems to me that's not what you're talking about...
>
> ~ Gijs
>
> ___
> 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: Visual Studio 2017 coming soon

[David Major]

This has reached mozilla-central and nightlies are now being built with
VS2017.

The clang-cl builds (which still rely on a VS package for link.exe, among
other things) remain on VS2015 while I work out some issues in clang-cl's
path detection. All other Windows jobs have moved to VS2017.

While I'm here, I want to give a shout out to everyone who helped with the
move to Taskcluster and in-tree job definitions. In previous compiler
upgrades I had to beg for a lot of handholding from busy releng folks. This
time was an absolute piece of cake. Everything was self-explanatory,
self-service, and easily testable on Try. Thank you!


On Wed, Oct 25, 2017 at 5:48 PM, David Major <dma...@mozilla.com> wrote:

> I'm planning to move production Windows builds to VS2017 (15.4.1) in bug
> 1408789.
>
> VS2017 has optimizer improvements that produce faster code. I've seen 3-6%
> improvement on Speedometer. There is also increased support for C++14 and
> C++17 language features: https://docs.microsoft.com/en-
> us/cpp/visual-cpp-language-conformance
>
> These days we tend not to support older VS for too long, so after some
> transition period you can probably expect that VS2017 will be required to
> build locally, ifdefs can be removed, etc. VS2017 Community Edition is a
> free download and it can coexist with previous compilers. Installation
> instructions are at: https://developer.mozilla.org/
> en-US/docs/Mozilla/Developer_guide/Build_Instructions/
> Windows_Prerequisites#Visual_Studio_2017
>
> If you have concerns, please talk to me or visit the bug. Thanks!
>
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Visual Studio 2017 coming soon

[David Major]

It would be great to get these speed gains for 58, hot on the heels of the
57 release.

My plan is this: if I can get this landed by Monday, that still leaves two
weeks in the cycle. Based on my positive experience thus far with this
compiler (this update has been much more smooth than past ones), I'm
comfortable with that number. If it goes longer than that, I agree it makes
sense to wait for a new train.



On Thu, Oct 26, 2017 at 3:31 AM, Sylvestre Ledru <sle...@mozilla.com> wrote:

> Hello,
>
>
> On 25/10/2017 23:48, David Major wrote:
> > I'm planning to move production Windows builds to VS2017 (15.4.1) in bug
> > 1408789.
> >
> In which version are you planning to land this change?
> As we are close to the end of the 58 cycle in nightly, it would be great
> to wait for 59.
>
> Thanks,
> Sylvestre
>
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Visual Studio 2017 coming soon

[David Major]

Agreed, changing compilers of an already-released ESR isn't a good idea.

You could use 2017 to build ESR52 locally though, if that's what you're
asking. Our tree has supported 2017 builds for a good while, since it's the
default VS download from Microsoft and a number of Mozillians have been
using it.

On Thu, Oct 26, 2017 at 10:33 AM, Jonathan Kew  wrote:

> On 26/10/2017 15:14, Milan Sreckovic wrote:
>
>> Are we locked into using the same compiler for the ESR updates?  In other
>> words, do we need to keep VS2015 for ESR52 builds until they are not needed
>> anymore?
>>
>>
> Yes, IMO.
>
> Whether or not we're "locked" in any technical sense, I think we should
> probably lock ourselves there by policy, unless a specific bug in the older
> compiler is directly affecting ESR builds in a serious way, and can only be
> solved by updating.
>
> Short of something like that (which seems pretty unlikely!), the stability
> risk involved in switching compilers doesn't sound like it belongs anywhere
> near the ESR world.
>
> JK
>
> ___
> 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

Visual Studio 2017 coming soon

[David Major]

I'm planning to move production Windows builds to VS2017 (15.4.1) in bug
1408789.

VS2017 has optimizer improvements that produce faster code. I've seen 3-6%
improvement on Speedometer. There is also increased support for C++14 and
C++17 language features:
https://docs.microsoft.com/en-us/cpp/visual-cpp-language-conformance

These days we tend not to support older VS for too long, so after some
transition period you can probably expect that VS2017 will be required to
build locally, ifdefs can be removed, etc. VS2017 Community Edition is a
free download and it can coexist with previous compilers. Installation
instructions are at:
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Windows_Prerequisites#Visual_Studio_2017

If you have concerns, please talk to me or visit the bug. Thanks!
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: C++ function that the optimizer won't eliminate

[David Major]

> On Fri, Oct 6, 2017 at 5:41 PM, David Major <dma...@mozilla.com> wrote:
> > I bet Google Benchmark will have what you want.
> >
> > As a first guess, maybe this?
> > https://github.com/google/benchmark/blob/master/include/
> benchmark/benchmark.h#L297
>
> Thank you. I guess it's the best to import that into the tree even
> though it seems to only consume values without returning a value.
> (I've been under the impression that it would be prudent to taint
> benchmark inputs as untrackable by the optimizer also.)
>
> I'm a bit surprised that the MSVC code path relies on an empty
> out-of-line function and not on inline asm.
>
>
MSVC doesn't allow inline assembly in 64-bit code.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: C++ function that the optimizer won't eliminate

[David Major]

I bet Google Benchmark will have what you want.

As a first guess, maybe this?
https://github.com/google/benchmark/blob/master/include/benchmark/benchmark.h#L297

(And if godbolt says they are wrong, please send them a PR :))


On Fri, Oct 6, 2017 at 9:16 AM, Gabriele Svelto  wrote:

> On 06/10/2017 11:00, Henri Sivonen wrote:
> > Do we already have a C++ analog of Rust's test::black_box() function?
> > I.e. a function that just passes through a value but taints it in such
> > a way that the optimizer can't figure out that there are no side
> > effects. (For the purpose of ensuring that the compiler can't
> > eliminate computation that's being benchmarked.)
> >
> > If we don't have one, how should one be written so that it works in
> > GCC, clang and MSVC?
> >
> > It's surprisingly hard to find an answer to this on Google or
> > StackOverflow, and experimentation on godbolt.org suggests that easy
> > answers that are found are also wrong.
> >
> > Specifically, this isn't the answer for GCC:
> > void* black_box(void* foo) {
> >   asm ("":"=r" (foo): "r" (foo):"memory");
> >   return foo;
> > }
>
> IIUC what you are looking for is the '+' constraint which implies the
> parameter is both read and written in the asm statement, e.g.:
>
> void* black_box(void* foo) {
>   asm ("":"+r" (foo): "r" (foo):"memory");
>   return foo;
> }
>
>  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: Actually-Infallible Fallible Allocations

[David Major]

I don't think that anyone deliberately set out to write the code this way.
Likely this is fallout from the mass-refactorings in bug 968520 and related
bugs. I'd recommend working with poiru and froydnj to see if there's any
automated follow-up we could do to remove/improve this pattern.

On Tue, Aug 1, 2017 at 12:31 PM, Alexis Beingessner  wrote:

> TL;DR: we're using fallible allocations without checking the result in
> cases where we're certain there is enough space already reserved. This is
> confusing and potentially dangerous for refactoring. Should we stop doing
> this?
>
> -
>
> 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);
>
> For those who aren't familiar with how our allocating APIs work:
>
> * by default we hard abort on allocation failure
> * but if you add `fallible` (or use the Fallible template), you will get
> back nullptr on allocation failure
>
> 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:
>
> * It's confusing to anyone who encounters this for the first time.
> * It's a time bomb for any bad refactoring which makes the allocations
> *actually* fallible
>
> I can however think of two reasons why this might be desirable
>
> * It encourages the optimizer to understand that the allocations can't
> fail, removing branches (dubious, these branches would predict perfectly if
> they exist at all)
>
> * It's part of a guideline/lint that mandates only fallible allocations be
> used in this part of the code.
>
> If the latter is a concern, I would much rather we use infallible
> allocations with a required comment, or some other kind of decoration to
> state "this is fine". In the absence of a checker that can statically prove
> the the AppendElement calls will never fail (which in the given example
> would in fact warn that we don't use `length` in the loop condition), I
> would rather mistakes lead to us crashing more, rather than Undefined
> Behaviour.
>
> Thoughts?
> ___
> 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: Care in the use of MOZ_CRASH_UNSAFE_* macros: data review needed

[David Major]

As of bug 1275780, rust panic text gets reported as a MOZ_CRASH reason.

On Mon, Jul 17, 2017 at 12:42 PM, Benjamin Smedberg 
wrote:

> I don't know really anything about how rust panics get reflected into
> crash-data. Who would be the right person to talk to about that?
>
> --BDS
>
> On Mon, Jul 17, 2017 at 12:40 PM, Emilio Cobos Álvarez 
> wrote:
>
> > On 07/17/2017 05:18 PM, Benjamin Smedberg wrote:> Unlike MOZ_CRASH,
> > which only annotates crashes with compile-time constants
> > > which are inherently not risky, both MOZ_CRASH_UNSAFE_OOL and
> > > MOZ_CRASH_UNSAFE_PRINTF can annotate crashes with arbitrary data. Crash
> > > reasons are publicly visible in crash reports, and in general it is not
> > > appropriate to send any kind of user data as part of the crash reason.
> >
> > I suppose the same happens with rust panics, should those also be
> reviewed?
> >
> >  -- Emilio
> > ___
> > 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: Improving visibility of compiler warnings

[David Major]

I'm not sure if this is exactly the same as the ALLOW_COMPILER_WARNINGS
that we use for warnings-on-errors, but FWIW as of a couple of months
ago I cleaned out the last warning-allowance in our "own" code.
ALLOW_COMPILER_WARNINGS usage is now only in external libraries (I'm
counting NSS and NSPR as "external" for this purpose since I can't land
code to m-c directly).

On Fri, May 19, 2017, at 02:55 PM, Kris Maglione wrote:
> I've actually been meaning to post about this, with some questions.
> 
> I definitely like that we now print warnings at the end of builds, since
> it 
> makes them harder to ignore. But the current amount of warnings spew at
> the 
> end of every build is problematic, especially when a build fails and I
> need 
> to scroll up several pages to find out why.
> 
> Can we make some effort to get clean warnings output at the end of
> standard 
> builds? A huge chunk of the warnings come from NSS and NSPR, and should
> be 
> easily fixable. Most of the rest seem to come from vendored libraries,
> and 
> should probably just be whitelisted if we can't get fixes upstreamed.
> 
> On Fri, May 19, 2017 at 11:44:08AM -0700, 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.
> >
> >There have been a few recent changes to increase the visibility of compiler
> >warnings with the expectation being that raising visibility will increase
> >the chances of someone addressing them. After all, a compiler warning is
> >either a) valid and should be fixed or b) invalid and should be ignored.
> >Either way, a compiler warning shouldn't exist.
> >
> >First, `mach build` now prints a list of all parsed compiler warnings at
> >the end of the build. More details are in the commit message:
> >https://hg.mozilla.org/mozilla-central/rev/4abe611696ab
> >
> >Second, Perfherder now records the compiler warning count as a metric. This
> >means we now have alerts when compiler warnings are added or removed, like
> >[1]. And, we can easily see graphs of compiler warnings over time, like
> >[2]. The compiler warnings are also surfaced in the "performance" tab of
> >build jobs on Treeherder, like [3].
> >
> >The Perfherder alerts and tracking are informational only: nobody will be
> >backing you out merely because you added a compiler warning. While the
> >possibility of doing that now exists, I view that decision as up to the C++
> >module. I'm not going to advocate for it. So if you feel passionately, take
> >it up with them :)
> >
> >Astute link clickers will note that the count metrics in CI are often noisy
> >- commonly fluctuating by a value of 1-2. I suspect there are race
> >conditions with interleaved process output and/or bugs in the compiler
> >warnings parser/aggregator. Since the values are currently advisory only,
> >I'm very much taking a "perfect is the enemy of good" attitude and have no
> >plans to improve the robustness.
> >
> >[1] https://treeherder.mozilla.org/perf.html#/alerts?id=6700
> >[2] https://mzl.la/2qFN0me and https://mzl.la/2rAkWR5
> >[3]
> >https://treeherder.mozilla.org/#/jobs?repo=autoland=100509284
> >___
> >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
> 
> The X server has to be the biggest program I've ever seen that doesn't
> do anything for you.
>   --Ken Thompson
> 
> ___
> 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

Gecko Profiler Win64 stability

[David Major]

Hi,

The Gecko Profiler used to be notoriously unstable on 64-bit Windows. (If
you're curious: it's mostly because the unwinding rules for Win64 require
the system libraries to do a lot of synchronization, which our stack walker
would often call in ways that could deadlock.)

Ever since the last Quantum Flow work week, I've been working on fixing
these issues. As of this week's nightlies, all of the hangs that have been
reported to me are resolved fixed. There might be some rare corner cases
still remaining, but I'm confident that the most frequent problems have
been addressed.

So my ask is: if you've previously been deterred from using the profiler on
Win64 builds, please give it another try! Flag me on any issues you find
and/or mark them blocking bug 1366030. I will treat these bugs with high
priority since I want to keep Quantum activities unblocked.

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

Re: A reminder about commit messages: they should be useful

[David Major]

I'd like to add to this a reminder that commit messages should describe
the _change_ and not the _symptom_. In other words, "Bug XYZ: Crash at
Foo::Bar" is not a good summary.

This is implied by what Boris said, but I've seen enough of these on my
pulsebot backscroll that it's worth mentioning explicitly.

Thanks!

On Mon, Apr 17, 2017, at 11:16 AM, Boris Zbarsky wrote:
> A quick reminder to patch authors and reviewers.
> 
> Changesets should have commit messages.  The commit message should 
> describe not just the "what" of the change but also the "why".  This is 
> especially true in cases when the "what" is obvious from the diff 
> anyway; for larger changes it makes sense to have a summary of the 
> "what" in the commit message.
> 
> As a specific example, if your diff is a one-line change that changes a 
> method call argument from "true" to "false", having a commit message 
> that says "change argument to mymethod from true to false" is not very 
> helpful at all.  A good commit message in this situation will at least 
> mention the meaning for the argument.  If that does not make it clear 
> why the change is being made, the commit message should explain the
> "why".
> 
> Thank you,
> Boris
> 
> P.S.  Yes, this was prompted by a specific changeset I saw.  This 
> changeset had been marked r+, which means neither the patch author not 
> the reviewer really thought about this problem.
> ___
> 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?

[David Major]

Try using Sysinternals Process Monitor to see what files MsMpEng.exe is
reading.

On Fri, Mar 17, 2017, at 04:26 PM, Ben Kelly wrote:
> Hi all,
> 
> I'm trying to configure my new windows build machine and noticed that
> builds were still somewhat slow.  I did:
> 
> 1) Put it in high performance power profile
> 2) Made sure my mozilla-central dir was not being indexed for search
> 3) Excluded my mozilla-central directory from windows defender
> 
> Watching the task monitor during a build, though, I still saw MsMpEng.exe
> (antivirus) running during the build.
> 
> I ended up added some very broad exclusions to get this down close to
> zero.  I am now excluding:
> 
>   - mozilla-central checkout
>   - mozilla-build install dir
>   - visual studio install dir
>   - /users/bkelly/appdada/local/temp
>   - /users/bkelly (because temp dir was not enough)
> 
> I'd like to narrow this down a bit.  Does anyone have a better list of
> things to exclude from virus scanning for our build process?
> 
> Thanks.
> 
> Ben
> ___
> 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

Renaming nsAString_internal to nsAString

[David Major]

TL;DR this will change crash signatures and possibly other tool output;
talk to me if this adversely affects you.

Now that bug 1332639 has removed the external string API, we no longer need
to differentiate between internal and external strings.

Currently we #define nsAString to nsAString_internal, so even though we
write nsAString in the source, the binary thinks the name is
nsAString_internal. Same for nsACString.

In bug 1346078 I plan to remove the #defines so that we just have
nsA(C)String in the binary.

If you have tools outside the tree that depend on the _internal names,
please let me know. (I'm already aware of the servo bindings and will
address those)

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

Re: Should &&/|| really be at the end of lines?

[David Major]

One thing I like about trailing operators is that they tend to match
what you'd find in bullet-point prose. Here's a made-up example:

You can apply for a refund of your travel insurance policy if:
* You cancel within 7 days of purchase, and
* You have not yet begun your journey, and
* You have not used any benefits of the plan.

Over time my eyes have come to expect the conjunction on the right.

On Fri, Feb 17, 2017, at 07:28 PM, gsquel...@mozilla.com wrote:
> While it's good to know how many people are for or against it, so that we
> get a sense of where the majority swings, I'd really like to know *why*
> people have their position. (I could learn something!)
> 
> So Nick, would you have some reasons for your "strong preference"? And
> what do you think of the opposite rationale as found in [2]?
> 
> (But I realize it's more work, so no troubles if you don't have the time
> to expand on your position here thank you for your feedback so far.)
> 
> On Friday, February 17, 2017 at 5:16:42 PM UTC+11, Nicholas Nethercote
> wrote:
> > I personally have a strong preference for operators at the end of lines.
> > The codebase seems to agree with me, judging by some rough grepping ('fff'
> > is an alias I have that's roughly equivalent to rgrep):
> > 
> > $ fff "&&$" | wc -l
> >   28907
> > $ fff "^ *&&" | wc -l
> >3751
> > 
> > $ fff "||$" | wc -l
> >   26429
> > $ fff "^ *||" | wc -l
> >2977
> > 
> > $ fff " =$" | wc -l
> >   39379
> > $ fff "^ *= " | wc -l
> > 629
> > 
> > $ fff " +$" | wc -l
> >   31909
> > $ fff "^ *+$" | wc -l
> > 491
> > 
> > $ fff " -$" | wc -l
> >2083
> > $ fff "^ *-$" | wc -l
> >  52
> > 
> > $ fff " ==$" | wc -l
> >   1501
> > $ fff "^ *== " | wc -l
> >   161
> > 
> > $ fff " !=$" | wc -l
> >   699
> > $ fff "^ *!= " | wc -l
> >   129
> > 
> > They are rough regexps and probably have some false positives, but the
> > numbers aren't even close; operators at the end of the line clearly
> > dominate.
> > 
> > I will conform for the greater good but silently weep inside every time I
> > see it.
> > 
> > Nick
> > 
> > On Fri, Feb 17, 2017 at 8:47 AM,  wrote:
> > 
> > > Question of the day:
> > > When breaking overlong expressions, should &&/|| go at the end or the
> > > beginning of the line?
> > >
> > > TL;DR: Coding style says 'end', I think we should change it to
> > > 'beginning' for better clarity, and consistency with other operators.
> > >
> > >
> > > Our coding style reads:
> > > "Break long conditions after && and || logical connectives. See below for
> > > the rule for other operators." [1]
> > > """
> > > Overlong expressions not joined by && and || should break so the operator
> > > starts on the second line and starts in the same column as the start of 
> > > the
> > > expression in the first line. This applies to ?:, binary arithmetic
> > > operators including +, and member-of operators (in particular the .
> > > operator in JavaScript, see the Rationale).
> > >
> > > Rationale: operator at the front of the continuation line makes for faster
> > > visual scanning, because there is no need to read to end of line. Also
> > > there exists a context-sensitive keyword hazard in JavaScript; see bug
> > > 442099, comment 19, which can be avoided by putting . at the start of a
> > > continuation line in long member expression.
> > > """ [2]
> > >
> > >
> > > I initially focused on the rationale, so I thought *all* operators should
> > > go at the front of the line.
> > >
> > > But it seems I've been living a lie!
> > > &&/|| should apparently be at the end, while other operators (in some
> > > situations) should be at the beginning.
> > >
> > >
> > > Now I personally think this just doesn't make sense:
> > > - Why the distinction between &&/|| and other operators?
> > > - Why would the excellent rationale not apply to &&/||?
> > > - Pedantically, the style talks about 'expression *not* joined by &&/||,
> > > but what about expression that *are* joined by &&/||? (Undefined 
> > > Behavior!)
> > >
> > > Based on that, I believe &&/|| should be made consistent with *all*
> > > operators, and go at the beginning of lines, aligned with the first 
> > > operand
> > > above.
> > >
> > > And therefore I would propose the following changes to the coding style:
> > > - Remove the lonely &&/|| sentence at [1].
> > > - Rephrase the first sentence at [2] to something like: "Overlong
> > > expressions should break so that the operator starts on the following 
> > > line,
> > > in the same column as the first operand for that operator. This applies to
> > > all binary operators, including member-of operators (in particular the .
> > > operator in JavaScript, see the Rationale), and extends to ?: where the 
> > > 2nd
> > > and third operands should be on separate lines and start in the same 
> > > column
> > > as the first operand."
> > > - Keep the rationale at [2].
> > >
> > > Also, I think we should add something about where to break expressions
> > > 

Re: Key measurements from memory reports are now shown by crash-stats.m.o

[David Major]

Nice!

I see that these fields are available in Super Search already, which is
great. This is going to make search queries really powerful.

On Tue, Feb 14, 2017, at 12:37 PM, Nicholas Nethercote wrote:
> Hi,
> 
> For a long time we have collected a memory report for most crash reports
> where the crash is caused by an out-of-memory problem. But the memory
> report data hasn't been incorporated nicely into crash-stats -- you have
> to
> save the memory report file to disk and then load it in about:memory to
> see
> anything.
> 
> I'm happy to say that things have now improved, via bug 1291173. For
> crash
> reports that contain a memory report, the following 16 key measurements
> are
> now shown in the default "Details" tab at crash-stats.m.o:
> 
> - explicit
> - gfx-textures
> - ghost-windows
> - heap-allocated
> - heap-overhead
> - heap-unclassified
> - host-object-urls
> - images
> - js-main-runtime
> - private
> - resident
> - resident-unique
> - system-heap-allocated
> - top(none)/detached
> - vsize
> - vsize-max-contiguous
> 
> See
> https://crash-stats.mozilla.org/report/index/c012f28b-cc09-48ba-9cfd-1e6c32170202
> for an example.
> 
> This change should give us better insight into causes of OOM crashes. For
> example, we will be able to see correlations between suspicious
> measurements (e.g. ghost-windows > 0) and add-ons.
> 
> Many thanks to Adrian Gaudebert for lots of help getting this working.
> 
> 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: Changing the representation of rectangles in platform code

[David Major]

Is there a specific problem that's being solved by this proposal? It would
be helpful to make this a bit more concrete, like "these benchmarks go x%
faster", or "here's a list of overflow bugs that will just vanish", or
"here's some upcoming work that this would facilitate".

On Thu, Feb 9, 2017 at 1:56 PM, Botond Ballo  wrote:

> Hi everyone!
>
> I would like to propose changing the internal representation of
> rectangles in platform code.
>
> We currently represent a rectangle by storing the coordinates of its
> top-left corner, its width, and its height. I'll refer to this
> representation as "x/y/w/h".
>
> I would like to propose storing instead the coordinates of the
> top-left corner, and the coordinates of the bottom-right corner. I'll
> refer to this representation as "x1/y1/x2/y2".
>
> The x1/y1/x2/y2 representation has several advantages over x/y/w/h:
>
>   - Several operations are more efficient with x1/y1/x2/y2, including
> intersection,
> union, and point-in-rect.
>   - The representation is more symmetric, since it stores two quantities
> of the
> same kind (two points) rather than a point and a dimension
> (width/height).
>   - The representation is less susceptible to overflow. With x/y/w/h,
> computation
> of x2/y2 can overflow for a large range of values of x/y and w/h.
> However,
> with x1/y1/x2/y2, computation of w/h cannot overflow if the
> coordinates are
> signed and the resulting w/h is unsigned.
>
> A known disadvantage of x1/y1/x2/y2 is that translating the rectangle
> requires translating both points, whereas translating x/y/w/h only
> requires translating one point. I think this disadvantage is minor in
> comparison to the above advantages.
>
> The proposed change would affect the class mozilla::gfx::BaseRect, and
> classes that derive from it (such as CSSRect, LayoutRect, etc., and,
> notably, nsRect and nsIntRect), but NOT other rectangle classes like
> DOMRect.
>
> I would like to propose making the transition as follows:
>
>   - Replace direct accesses to the 'width' and 'height' fields throughout
> the codebase with calls to getter and setter methods. (There aren't
> that many - on the order of a few dozen, last I checked.)
>
>   - Make the representation change, which is non-breaking now that
> the direct accesses to 'width' and 'height' have been removed.
>
>   - Examine remaining calls to the getters and setters for width and
> height and see if any can be better expressed using operations
> on the points instead.
>
> The Graphics team, which owns this code, is supportive of this change.
> However, since this is a fundamental utility type that's used by a
> variety of platform code, I would like to ask the wider platform
> development community for feedback before proceeding. Please let me
> know if you have any!
>
> Thanks,
> Botond
>
> [1] http://searchfox.org/mozilla-central/rev/
> 672c83ed65da286b68be1d02799c35fdd14d0134/gfx/2d/BaseRect.h#46
> ___
> 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: Please do not add any new CppUnitTests/GeckoCppUnitTests

[David Major]

On Wed, Nov 9, 2016, at 06: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

+1: When testing new compilers I find it really valuable to have an
assortment of tiny programs in the tree. It lets me work through the
first few layers of bugs in isolation, before diving into the complexity
of libxul. It would be helpful to keep at least some of the CppUnitTests
around in a lightweight form.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switch to Google C++ Style Wholesale (was Re: Proposal to remove `aFoo` prescription from the Mozilla style guide for C and C++)

[David Major]

May I request that the major parts of this not happen until we have a
blame that can see through such changes.

Last I checked, gps had some ideas in that space but lacked time to
implement.

On Wed, Jul 15, 2015, at 03:23 AM, Benjamin Smedberg wrote:
 
 
 On 7/8/2015 7:31 AM, Nathan Froyd wrote:
  If somebody is willing to do the formatting, I'm willing to do the 
  review. I think the thread has reached the point of people repeating 
  ad nauseum what was already said earlier in the thread, so it's time 
  for a decision. Benjamin?
 
 Aww, I was avoiding getting into this thread.
 
 I personally have no strong preference, and our existing community is 
 pretty deeply divided. I doubt we're going to come to consensus here, 
 and this is a pretty tough decision to make on its own. I do believe 
 that consistency trumps module/personal preference in terms of coding
 style.
 
 The argument I am most sympathetic to is that this convention is a 
 barrier to new contributors. Making new contributors productive, both 
 employees and volunteers, is a very good reason to choose one style over 
 another.
 
 Given that premise, we shouldn't just change aArgument; we should adopt 
 the Google C++ style guide wholesale:
 
 * names_with_underscores
 * members_with_trailing_
 * no more ns prefix
 
 There is good research that underscore_names are more readable, and many 
 of these will be more familiar to new contributors. Also we have a fair 
 bit of shared code with Google.
 
 If there is a decision to be made here, I'd like to make this RFC:
 
 * switch our codebase wholesale to the Google C++ style guide
 
 With the following implementation plan:
 
 * For now, code should continue to be written in the current style with 
 aFoo, mFoo, and camelCase.
 * get our code -Wshadow clean
 * Ask poiru to investigate auto-renaming of our variables including 
 mFoo, aFoo, and camelCase to the google-standard local variable names.
 * Do not make any changes to the style guide or standard practice until 
 we're comfortable that we can do automatic changes.
 * Make the automatic changes and change our style guide at roughly the 
 same time.
 * Go back and deal with class names (nsFoo) as a separate/later pass.
 
 --BDS
 
 ___
 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: Shutdown hangs are very common

[David Major]

 Should fixing shutdown hangs be higher priority?
 And if so, should we allow features with shutdown hangs to be released?

I admit to skimming over shutdown-hang signatures when looking at
topcrash lists. In my experience they're often longstanding issues that
are difficult to diagnose and rarely end in a successful fix. Given
infinite resources, we should absolutely work on these hangs. In
practice though, we always have more bugs than time, so I tend to focus
my efforts on other topcrashes that we have a better chance of fixing.

A possible exception is for new, never-before-seen shutdown hangs. Those
are likely to be more actionable since we often have a clear regression
range. I think it would be fair to consider those a blocker for the
associated feature.

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

Re: About the bitfield requirement for portibility

[David Major]

Mixing types will likely produce unexpected and/or wasteful results. Examples 
at 
http://randomascii.wordpress.com/2010/06/06/bit-field-packing-with-visual-c/. I 
believe that is still true in VS2013.

David

- Original Message -
 From: Xidorn Quan quanxunz...@gmail.com
 To: dev-platform@lists.mozilla.org
 Cc: David Baron dba...@dbaron.org, s...@mozilla.org, blizz...@mozilla.org
 Sent: Tuesday, October 21, 2014 4:21:21 PM
 Subject: About the bitfield requirement for portibility
 
 Hi,
 
 I read the C++ portibility guide [1], in which it is said that all
 bitfields should have the same type, or some compiler may mishandle the
 code. Is that still true for the compiler set we currently use? The
 compiler the doc mentioned is MSVC++8 which I believe we have dropped. Can
 we use different type for bitfields in a class in our new code?
 
 I found the document is quiet old. It doesn't seem to be maintained
 anymore. The last update was in 7 years ago, and there are many rules have
 been broken in our current code. At least many bitfield-related rules.
 Should we follow the guide there? Or do we have another document for this
 purpose?
 
 [1]: https://developer.mozilla.org/en-US/docs/Mozilla/Cpp_portability_guide
 
 Regards,
 Xidorn
 ___
 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: Compiler version expectations

[David Major]

I was thinking it would be nice to support VS2010 as long as any of our main 
channels use it -- meaning we could drop it on the first day of 39. But I have 
no practical justification for that. If it causes a burden on Skia work then it 
might be reasonable to switch sooner.

 This set: http://chromium-cpp.appspot.com/
What MS compiler does that list require? There's a number of people building 
with VS2012, would that still be supported?

David

- Original Message -
 From: Jeff Muizelaar jmuizel...@mozilla.com
 To: Ehsan Akhgari ehsan.akhg...@gmail.com
 Cc: dev-platform@lists.mozilla.org list dev-platform@lists.mozilla.org
 Sent: Friday, October 17, 2014 9:14:19 AM
 Subject: Re: Compiler version expectations
 
 
 On Oct 16, 2014, at 3:57 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote:
 
  On 2014-10-16, 3:49 PM, Jeff Muizelaar wrote:
  After some discussion some IRC it was clear that our compiler deprecation
  schedule is not very clear.
  
  Now that we’re using VS2013 on trunk and will soon not being using GCC 4.4
  for B2G, I expect we’ll be dropping support for building with VS2010 and
  GCC 4.4  in the near term.
  
  GCC is https://bugzilla.mozilla.org/show_bug.cgi?id=1077549.  No specific
  bug or plans for MSVC2010, but I'd be open to killing support for it on
  the next release train.
  
  This is important to us because Skia is planing on using more C++11
  features in the near term and we’d like to continue updating from
  upstream. Are there reasons we can’t drop support for these compilers in
  the 37-38 time frame?
  
  What C++11 features specifically?
 
 This set: http://chromium-cpp.appspot.com/
 
 -Jeff
 
 ___
 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: Using __declspec(thread) on Windows

[David Major]

 in the .EXE or in DLLs statically linked by the .EXE, so not libxul, but in
 our shipped Windows builds mozglue.dll is statically linked to firefox.exe
 so we could put __declspec(thread) variables there.

What does 'statically linked' mean in this context? Mozglue.dll is still a DLL, 
but yes it's a load-time link in version 33. Starting in version 34 it's a 
delay-load due to some WinXP sorcery.

- Original Message -
 From: Robert O'Callahan rob...@ocallahan.org
 To: dev-platform@lists.mozilla.org
 Sent: Friday, October 17, 2014 10:10:57 AM
 Subject: Using __declspec(thread) on Windows
 
 It would be cool to use fast TLS via __declspec(thread) on Windows (and
 __thread on gcc/clang). Due to WinXP bustage that only works for variables
 in the .EXE or in DLLs statically linked by the .EXE, so not libxul, but in
 our shipped Windows builds mozglue.dll is statically linked to firefox.exe
 so we could put __declspec(thread) variables there.
 
 However, that would break if someone tried to dynamically load libxul on
 WinXP in the context of a .EXE which did not statically link mozglue.dll.
 Does anyone know if that's a problem? I assume no-one's finding the Firefox
 libxul.dll and loading it from their own .EXE, but maybe they're building
 their own? Do we care?
 
 Rob
 --
 oIo otoeololo oyooouo otohoaoto oaonoyooonoeo owohooo oioso oaonogoroyo
 owoiotoho oao oboroootohoeoro oooro osoiosotoeoro owoiololo oboeo
 osouobojoeocoto otooo ojouodogomoeonoto.o oAogoaoiono,o oaonoyooonoeo
 owohooo
 osoaoyoso otooo oao oboroootohoeoro oooro osoiosotoeoro,o o‘oRoaocoao,o’o
 oioso
 oaonosowoeoroaoboloeo otooo otohoeo ocooouoroto.o oAonodo oaonoyooonoeo
 owohooo
 osoaoyoso,o o‘oYooouo ofolo!o’o owoiololo oboeo oiono odoaonogoeoro
 ooofo
 otohoeo ofoioroeo ooofo ohoeololo.
 ___
 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: Switching to Visual Studio 2013

[David Major]

VS2013 is now on inbound and all Windows builds are green.

(Win64 builds were actually switched late last week, as they are
unaffected by trains.)

Please file bugs blocking 914596 if you encounter any VS2013-specific
issues.

If you're not sure what version you're running, you can:
* Check the package:
  VS2013 builds contain msvcr120.dll and msvcp120.dll.
  VS2010 builds contain msvcr100.dll and msvcp100.dll.
* Check the build log:
  VS2013 logs contain the string 'Compiler Version 18.00'.
  VS2010 logs contain the string 'Compiler Version 16.00'.

David


- Original Message -
 From: David Major dma...@mozilla.com
 To: dev-platform@lists.mozilla.org, firefox-...@mozilla.org
 Sent: Wednesday, October 8, 2014 7:21:07 PM
 Subject: Re: Switching to Visual Studio 2013
 
 Update: the switch will happen early in version 36.
 
 The build machines needed a reinstall of VS2013 due to a deployment issue. As
 merge day is coming up, it's not a good time for major changes. We'll wait
 for 36 to have more bake time.
 
 On the plus side, we picked up Update 3 in the process.
 
 David
 
 - Original Message -
  From: David Major dma...@mozilla.com
  To: dev-platform@lists.mozilla.org, firefox-...@mozilla.org
  Sent: Friday, August 22, 2014 6:27:58 PM
  Subject: Switching to Visual Studio 2013
  
  We plan to switch the Windows build machines to Visual Studio 2013 on the
  Firefox 35 train.
  
  Some benefits from this change:
  * No more linker OOM crashes. VS2013 includes a 64-bit toolchain for 32-bit
  builds, so the linker will no longer be limited to 4GB address space.
  * The linker capacity opens the door for merging our binaries into libxul
  (like we do on the other platforms)
  * More than 2x improvement in PGO build times
  * Better language support
  
  The tracking bug is 914596. The remaining blockers are here:
  https://bugzilla.mozilla.org/showdependencytree.cgi?id=914596hide_resolved=1
  
  David
  
 ___
 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: Switching to Visual Studio 2013

[David Major]

Update: the switch will happen early in version 36.

The build machines needed a reinstall of VS2013 due to a deployment issue. As 
merge day is coming up, it's not a good time for major changes. We'll wait for 
36 to have more bake time. 

On the plus side, we picked up Update 3 in the process.

David

- Original Message -
 From: David Major dma...@mozilla.com
 To: dev-platform@lists.mozilla.org, firefox-...@mozilla.org
 Sent: Friday, August 22, 2014 6:27:58 PM
 Subject: Switching to Visual Studio 2013
 
 We plan to switch the Windows build machines to Visual Studio 2013 on the
 Firefox 35 train.
 
 Some benefits from this change:
 * No more linker OOM crashes. VS2013 includes a 64-bit toolchain for 32-bit
 builds, so the linker will no longer be limited to 4GB address space.
 * The linker capacity opens the door for merging our binaries into libxul
 (like we do on the other platforms)
 * More than 2x improvement in PGO build times
 * Better language support
 
 The tracking bug is 914596. The remaining blockers are here:
 https://bugzilla.mozilla.org/showdependencytree.cgi?id=914596hide_resolved=1
 
 David
 
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Switching to Visual Studio 2013

[David Major]

Please open bugs for this kind of thing. I filed 1061339 for you.

- Original Message -
 From: xunxun xunxun1...@gmail.com
 To: David Major dma...@mozilla.com
 Cc: dev-platform@lists.mozilla.org, firefox-...@mozilla.org
 Sent: Monday, September 1, 2014 9:01:19 PM
 Subject: Re: Switching to Visual Studio 2013
 
 And after switching to VS2013, we should enable AVX2 on libvpx then,
 because libvpx has some AVX2 optimization code.
 
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Switching to Visual Studio 2013

[David Major]

We plan to switch the Windows build machines to Visual Studio 2013 on the 
Firefox 35 train.

Some benefits from this change:
* No more linker OOM crashes. VS2013 includes a 64-bit toolchain for 32-bit 
builds, so the linker will no longer be limited to 4GB address space.
* The linker capacity opens the door for merging our binaries into libxul (like 
we do on the other platforms)
* More than 2x improvement in PGO build times
* Better language support

The tracking bug is 914596. The remaining blockers are here: 
https://bugzilla.mozilla.org/showdependencytree.cgi?id=914596hide_resolved=1

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

Re: People building and debugging Firefox on Windows wanted

[David Major]

My improvements were closer to 5%. I use VS2013's amd64_x86 cross-compiler. I 
didn't touch the -FS flag (I imagine it just becomes meaningless).

Debug build, before: build 17:06, objdir 3.83GB
Debug build, -Z7:build 16:06, objdir 5.22GB
Opt build, before:   build 17:47, objdir 3.01GB
Opt build, -Z7:  build 16:59, objdir 4.43GB

I used the opt Z7 build for a bug investigation and, at least at first glance, 
I didn't notice any problems with the debugging information.

David

- Original Message -
 From: Mike Hommey m...@glandium.org
 To: Neil n...@parkwaycc.co.uk
 Cc: dev-platform@lists.mozilla.org
 Sent: Thursday, April 24, 2014 11:04:35 PM
 Subject: Re: People building and debugging Firefox on Windows wanted
 
 On Thu, Apr 24, 2014 at 10:19:11AM +0100, Neil wrote:
  Mike Hommey wrote:
  
 mk_add_options export MOZ_DEBUG_FLAGS=-Z7
  
  -Z7 is faster than -Zi?
 
 Surprisingly, yes.
 
  Do VS2013 users need to turn off -FS?
 
 Maybe, although it may just be ignored if using -Z7.
 
 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: Reacting more strongly to low-memory situations in Firefox 25

[David Major]

It seems that the 12MB reservation was aborting due to an invalid parameter. 
I've filed bug 943051.

- Original Message -
From: Benjamin Smedberg benja...@smedbergs.us
To: Ehsan Akhgari ehsan.akhg...@gmail.com, dev-platform@lists.mozilla.org
Sent: Monday, November 25, 2013 9:18:02 AM
Subject: Re: Reacting more strongly to low-memory situations in Firefox 25

On 11/25/2013 12:11 PM, Ehsan Akhgari wrote:

 Do we know how much memory we tend to use during the minidump 
 collection phase?
No, we don't. It seems that the Windows code maps all of the DLLs into 
memory again in order to extract information from them.
   Does it make sense to try to reserve an address space range large 
 enough for those allocations, and free it up right before trying to 
 collect a crash report to make sure that the crash reporter would not 
 run out of (V)memory in most cases?
We already do this with a 12MB reservation, which had no apparent effect 
(bug 837835).

--BDS

___
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: Audit your code if you use mozilla::WeakPtr

[David Major]

Isn't it ultimately up to the developer to get it right? Someone could just as 
well forget to use |if (object)| from your example.

Here's a sample usage from the header file:
 *   // Test a weak pointer for validity before using it.
 *   if (weak) {
 * weak-num = 17;
 * weak-act();
 *   }

- Original Message -
From: Ehsan Akhgari ehsan.akhg...@gmail.com
To: dev-platform@lists.mozilla.org
Sent: Tuesday, October 8, 2013 3:54:17 PM
Subject: Audit your code if you use mozilla::WeakPtr

I and Benoit Jacob discovered a bug in WeakPtr (bug 924658) which makes its
usage unsafe by default.  The issue is that WeakPtr provides convenience
methods such as operator-, which mean that the consumer can directly
dereference it without the required null checking first.  This means that
you can have code like the below:

WeakPtrClass foo = realObject-asWeakPtr();
// ...
foo-Method();

That will happily compile and will crash at runtime if the object behind
the weak pointer is dead.  The correct way of writing such code is:

Class* object = foo.get();
if (object) {
  object-Method();
}

I don't know enough about all of the places which use WeakPtr myself to fix
them all, but if you have code using this in your module, please spend some
time auditing the code, and fix it.  Please file individual bugs for your
components blocking bug 924658.

Thanks!
--
Ehsan
http://ehsanakhgari.org/
___
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: Audit your code if you use mozilla::WeakPtr

[David Major]

Does it? I don't think I'm any more or less likely to omit a validity check 
using operator-() vs get(). Maybe it's just me.

It seems like get() might actually be *more* prone to failure. Imagine:

Class* object = foo.get();
if (object) {
  object-Method();
}
// ... A lot of stuff happens and the ref blows up ...
if (object) {
  object-Method(); // oops
}


- Original Message -
From: Ehsan Akhgari ehsan.akhg...@gmail.com
To: David Major dma...@mozilla.com
Cc: dev-platform@lists.mozilla.org
Sent: Tuesday, October 8, 2013 4:27:03 PM
Subject: Re: Audit your code if you use mozilla::WeakPtr

On 2013-10-08 7:10 PM, David Major wrote:
 Isn't it ultimately up to the developer to get it right? Someone could just 
 as well forget to use |if (object)| from your example.

 Here's a sample usage from the header file:
   *   // Test a weak pointer for validity before using it.
   *   if (weak) {
   * weak-num = 17;
   * weak-act();
   *   }

Sure, but that convenience operator makes it natural to write 
incorrect code by default.  It's better to be explicit and correct, than 
implicit and wrong.  :-)

Cheers,
Ehsan

 - Original Message -
 From: Ehsan Akhgari ehsan.akhg...@gmail.com
 To: dev-platform@lists.mozilla.org
 Sent: Tuesday, October 8, 2013 3:54:17 PM
 Subject: Audit your code if you use mozilla::WeakPtr

 I and Benoit Jacob discovered a bug in WeakPtr (bug 924658) which makes its
 usage unsafe by default.  The issue is that WeakPtr provides convenience
 methods such as operator-, which mean that the consumer can directly
 dereference it without the required null checking first.  This means that
 you can have code like the below:

 WeakPtrClass foo = realObject-asWeakPtr();
 // ...
 foo-Method();

 That will happily compile and will crash at runtime if the object behind
 the weak pointer is dead.  The correct way of writing such code is:

 Class* object = foo.get();
 if (object) {
object-Method();
 }

 I don't know enough about all of the places which use WeakPtr myself to fix
 them all, but if you have code using this in your module, please spend some
 time auditing the code, and fix it.  Please file individual bugs for your
 components blocking bug 924658.

 Thanks!
 --
 Ehsan
 http://ehsanakhgari.org/
 ___
 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