Re: Proposal: remove support for running desktop Firefox in single-process mode (e10s disabled) anywhere but in tests
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
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
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
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 a1
Re: Improving our usage of Bugzilla
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...
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...
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++
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
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
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
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
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&originalRevision=2adb3cb404ad&newProject=try&newRevision=5336df94a9ce&framework=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
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)
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
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
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
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
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
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
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
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 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 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
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
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
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 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
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 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
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
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
> On Fri, Oct 6, 2017 at 5:41 PM, David Major 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
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
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 &mSocket = *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 &mSocket =
> *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
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
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&selectedJob=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
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
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?
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
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?
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&now; 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&others 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
> > > with oper
Re: Key measurements from memory reports are now shown by crash-stats.m.o
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
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
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++)
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
> 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
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" > To: dev-platform@lists.mozilla.org > Cc: "David Baron" , 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: Using __declspec(thread) on Windows
> 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" > 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: Compiler version expectations
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" > To: "Ehsan Akhgari" > Cc: "dev-platform@lists.mozilla.org list" > Sent: Friday, October 17, 2014 9:14:19 AM > Subject: Re: Compiler version expectations > > > On Oct 16, 2014, at 3:57 PM, Ehsan Akhgari 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: Switching to Visual Studio 2013
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" > 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" > > 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=914596&hide_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
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" > 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=914596&hide_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
Please open bugs for this kind of thing. I filed 1061339 for you. - Original Message - > From: "xunxun" > To: "David Major" > 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
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=914596&hide_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
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" > To: "Neil" > 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
It seems that the 12MB reservation was aborting due to an invalid parameter. I've filed bug 943051. - Original Message - From: "Benjamin Smedberg" To: "Ehsan Akhgari" , 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
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"
To: "David Major"
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"
> 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:
>
> WeakPtr 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
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"
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:
WeakPtr 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
