Re: Removing "MathML View Source" menu item? [was: Can we remove nsIEntityConverter?]

[Frédéric Wang]

Le 17/05/2017 à 17:14, J. Ryan Stinnett a écrit :
> Looking at the add-on, it seems to miss one feature: Currently in m-c, you
> can select a portion of a MathML expression, choose "View Selection
> Source", and see the MathML source for the selection. If that's added to
> your add-on, I believe you'd have feature parity with the existing MathML
> view source support.

Right, I had opened
https://github.com/fred-wang/webextension-mathml-view-source/issues/7
for missing features. I don't know exactly how important they are, though.

> I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1365626 to remove
> MathML view source from m-c.

Thanks, maybe a first step would be to remove the usage of
nsIEntityConverter as Henri indicated as I don't think it's an important
feature (this is
https://github.com/fred-wang/webextension-mathml-view-source/issues/6).


-- 
Frédéric Wang
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Changing our thread APIs for Quantum DOM scheduling

[David Teller]

Out of curiosity, how will this interact with nsCOMPtr thread-safe (or
thread-unsafe) refcounting?

Also, in code I have seen, `NS_IsMainThread` is used mainly for
assertion checking. I *think* that the semantics you detail below will
work, but do you know if there is a way to make sure of that?

Also, I had the impression that Quantum DOM scheduling made JS event
loop spinning unncessary. Did I miss something?

Cheers,
 David

On 5/19/17 1:38 AM, Bill McCloskey wrote:
> Hi everyone,
> 
> One of the challenges of the Quantum DOM project is that we will soon have
> multiple "main" threads [1]. These will be real OS threads, but only one of
> them will be allowed to run code at any given time. We will switch between
> them at well-defined points (currently just the JS interrupt callback).
> This cooperative scheduling will make it much easier to keep our global
> state consistent. However, having multiple "main" threads is likely to
> cause confusion.
> 
> To simplify things, we considered trying to make these multiple threads
> "look" like a single main thread at the API level, but it's really hard to
> hide something like that. So, instead, we're going to be transitioning to
> APIs that try to avoid exposing threads at all. This post will summarize
> that effort. You can find more details in this Google doc:
> 
> https://docs.google.com/document/d/1MZhF1zB5_dk12WRiq4bpmNZUJWmsIt9OTpFUWAlmMyY/edit?usp=sharing
> 
> [Note: I'd like this thread (and the Google doc) to be for discussing
> threading APIs. If you have more general questions about the project,
> please contact me personally.]
> 
> The main API change is that we're going to make it a lot harder to get hold
> of an nsIThread for the main thread. Instead, we want people to work with
> event targets (nsIEventTarget). The advantage of event targets is that all
> the main threads will share a single event loop, and therefore a single
> nsIEventTarget. So code that once expected a single main thread can now
> expect a single main event target.
> 
> The functions NS_GetMainThread, NS_GetCurrentThread, and
> do_Get{Main,Current}Thread will be deprecated. In their place, we'll
> provide mozilla::Get{Main,Current}ThreadEventTarget. These functions will
> return an event target instead of a thread.
> 
> More details:
> 
> NS_IsMainThread: This function will remain pretty much the same. It will
> return true on any one of the main threads and false elsewhere.
> 
> Dispatching runnables: NS_DispatchToMainThread will still work, and you
> will still be able to dispatch using Get{Main,Current}ThreadEventTarget.
> From JS, we want people to use Services.tm.dispatchToMainThread.
> 
> Thread-safety assertions: Code that used PR_GetCurrentThread for thread
> safety assertions will be converted to use NS_ASSERT_OWNINGTHREAD, which
> will allow code from different main threads to touch the same object.
> PR_GetCurrentThread will be deprecated. If you really want to get the
> current PRThread*, you can use GetCurrentPhysicalThread, which will return
> a different value for each main thread.
> 
> Code that uses NS_GetCurrentThread for thread safety assertions will be
> converted to use nsIEventTarget::IsOnCurrentThread. The main thread event
> target will return true from IsOnCurrentThread if you're on any of the main
> threads.
> 
> Nested event loop spinning: In the future, we want people to use
> SpinEventLoopUntil to spin a nested event loop. It will do the right thing
> when called on any of the main threads. We'll provide a similar facility to
> JS consumers.
> 
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Avoiding jank in async functions/promises?

[Andreas Farre]

On Fri, May 19, 2017 at 4:18 AM, Mark Hammond  wrote:
> On 5/18/17 12:03 PM, Boris Zbarsky wrote:
>>
>> On 5/17/17 9:22 PM, Mark Hammond wrote:
>>>
>>> I'm wondering if there are any ideas about how to solve this optimally?
>>
>>
>> I assume
>> https://w3c.github.io/requestidlecallback/#the-requestidlecallback-method
>> doesn't have quite the right semantics here?  That would let you run when
>> the browser is idle, and give you some idea of how long you can run for
>> before you should yield.
>
>
> I didn't quite expect this to work, but by abusing rIC I can almost make
> something work - I just have the callback stash the IdleDeadline object and
> return immediately, but continue to refer to it anyway. eg:
>
> let idleChecker = {
>   resolved: Promise.resolve(),
>   deadline: null,
>   promiseIdle() {
> if (this.deadline && this.deadline.timeRemaining() > 0) {
>   return this.resolved;
> }
> this.deadline = null
> return new Promise(resolve => {
>   window.requestIdleCallback(deadline => {
> this.deadline = deadline;
> resolve();
>   });
> })
>   }
> }
>
> async function janky() {
>   let start = Date.now();
>   for (let i = 0; i < 1000; i++) {
> await Promise.resolve();
> await idleChecker.promiseIdle();
>   }
>   console.log("took", Date.now() - start)
> }
> janky().then(() => console.log("done"));
>
> I 1/2 expect this to defeat the intent/implementation of rIC, but it does
> work, causing ~2x-5x slowdown of the loop. I wonder if this is worth
> experimenting with some more?
>
> Mark
>

So if you have a look at how the idle callback algorithm is defined[1]
and what timeRemaining is supposed to return[2] you see that
timeRemaining doesn't update its sense of idleness, it only concerns
itself with the deadline. So if you save the IdleDeadline object and
resolve early, then timeRemaining won't know that the idle period
entered upon calling the idle callback might have ended.

I do think that you need to invert this somehow, actually doing the
work inside a rIC callback. Something like[3]:

  let idleTask = {
total: 10,
progress: 0,
doWork: async function(deadline) {
  for (; this.progress < this.total && deadline.timeRemaining() >
0; ++this.progress) {
await Promise.resolve();
  }
  console.log(this.total - this.progress)
},
schedule(deadline) {
  this.doWork(deadline).then(() => {
if (this.progress < this.total) {
  requestIdleCallback(this.schedule.bind(this));
} else {
  this.resolve();
}
  })
},
run() {
  this.resolve = null;
  return new Promise(resolve => {
this.resolve = resolve;
requestIdleCallback(this.schedule.bind(this));
  })
}
  }

  async function janky() {
var start = Date.now();
await idleTask.run();
console.log("took", Date.now() - start)
  }

  var handle = setInterval(function (){ console.log("I shouldn't be
blocked!")}, 500)
  janky().then(() => console.log("janky done")).then(() =>
clearTimeout(handle));

perhaps? The doWork function is indeed async, and executed within a
loop that both handles how much work that should be done as well as
using IdleDeadline.timeRemaining as expected.

[1] https://w3c.github.io/requestidlecallback/#invoke-idle-callbacks-algorithm
[2] https://w3c.github.io/requestidlecallback/#the-timeremaining-method

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

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

[Nicholas Nethercote]

There's also a pre-processor constant that we define in Valgrind/ASAN/etc.
builds that you can check in order to free more stuff than you otherwise
would. But I can't for the life of me remember what it's called :(

Nick

On Sat, May 20, 2017 at 5:09 AM, Jeff Muizelaar 
wrote:

> We use functions like cairo_debug_reset_static_data() on shutdown to
> handle cases like this.
>
> -Jeff
>
> On Fri, May 19, 2017 at 1:44 AM, Henri Sivonen 
> wrote:
> > On Tue, May 16, 2017 at 7:03 AM, Tim Guan-tin Chien
> >  wrote:
> >> According to Alexa top 100 Taiwan sites and quick spot checks, I can
> only
> >> see the following two sites encoded in Big5:
> >>
> >> http://www.ruten.com.tw/
> >> https://www.momoshop.com.tw/
> >>
> >> Both are shopping sites (eBay-like and Amazon-like) so you get the idea
> how
> >> forms are used there.
> >
> > Thank you. It seems to me that encoder performance doesn't really
> > matter for sites like these, since the number of characters one would
> > enter in the search field at a time is very small.
> >
> >> Mike reminded me to check the Tax filing website:
> http://www.tax.nat.gov.tw/
> >> .Yes, it's unfortunately also in Big5.
> >
> > I guess I'm not going to try filing taxes there for testing. :-)
> >
> > - -
> >
> > One option I've been thinking about is computing an encode
> > acceleration table for JIS X 0208 on the first attempt to encode a CJK
> > Unified Ideograph in any of Shift_JIS, EUC-JP or ISO-2022-JP, for GBK
> > on the first attempt to encode a CJK Unified Ideograph in either GBK
> > or gb18030, and for Big5 on the first attempt to encode a CJK Unified
> > Ideograph in Big5.
> >
> > Each of the three tables would then remain allocated through to the
> > termination of the process.
> >
> > This would have the advantage of not bloating our binary footprint
> > with data that can be computed from other data in the binary while
> > still making legacy Chinese and Japanese encode fast without a setup
> > cost for each encoder instance.
> >
> > The downsides would be that the memory for the tables wouldn't be
> > reclaimed if the tables aren't needed anymore (the browser can't
> > predict the future) and executions where any of the tables has been
> > created wouldn't be valgrind-clean. Also, in the multi-process world,
> > the tables would be recomputed per-process. OTOH, if we shut down
> > rendered processes from time to time, it would work as a coarse
> > mechanism to reclaim the memory is case Japanese or Chinese legacy
> > encode is a relatively isolated event in the user's browsing pattern.
> >
> > Creating a mechanism for the encoding library to become aware of
> > application shutdown just in order to be valgrind-clean would be
> > messy, though. (Currently, we have shutdown bugs where uconv gets used
> > after we've told it can shut down. I'd really want to avoid
> > re-introducing that class of bugs with encoding_rs.)
> >
> > Is it OK to create allocations that are intentionally never freed
> > (i.e. process termination is what "frees" them)? Is valgrind's message
> > suppression mechanism granular enough to suppress three allocations
> > from a particular Rust crate statically linked into libxul?
> >
> > --
> > Henri Sivonen
> > hsivo...@hsivonen.fi
> > https://hsivonen.fi/
> > ___
> > 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: Is it OK to make allocations that intentionally aren't freed? (was: Re: Is Big5 form submission fast enough?)

[Botond Ballo]

On Fri, May 19, 2017 at 10:38 PM, Nicholas Nethercote
 wrote:
> There's also a pre-processor constant that we define in Valgrind/ASAN/etc.
> builds that you can check in order to free more stuff than you otherwise
> would. But I can't for the life of me remember what it's called :(

It looks like some code checks for MOZ_ASAN and MOZ_VALGRIND.

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

Re: Gecko Profiler Win64 stability

[Michael Layzell]

You've done a fantastic job of making it more stable - thank you. I'll make
sure to flag you on any more synchronization issues I run into.

On Thu, May 18, 2017 at 3:04 PM, David Major  wrote:

> 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!
>
> ___
> 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: Intent to ship: NetworkInformation

[Ben Kelly]

Can the people who have concerns about the NetworkInformation API please
provide the feedback to google on this blink-dev thread:

https://groups.google.com/a/chromium.org/d/msg/blink-dev/UVfNMH50aaQ/CXY6S39TBQAJ

In particular, I think they tried to consider privacy in this part of the
spec:

https://wicg.github.io/netinfo/#privacy-considerations

Thanks.

Ben

On Fri, Dec 23, 2016 at 10:43 AM, Eric Rescorla  wrote:

> On Thu, Dec 22, 2016 at 10:31 PM,  wrote:
>
> > On Wednesday, December 21, 2016 at 12:51:10 AM UTC+11, Eric Rescorla
> wrote:
> > > I'm not really following this argument. Usually when a document has
> been
> > > floating
> > > around a long time but clearly has basic design issues and can't get
> > > consensus,
> > > even when a major vendor has implemented it, that's a sign that it
> > > *shouldn't*
> > > be standardized until those issues are resolved. That's not standards
> > > fatigue,
> > > it's the process working as designed.
> >
> > The API addresses the use cases, but people here see those use cases as
> > too basic because they don't represent average users (e.g., Boris'
> somewhat
> > esoteric network setup). Most people have wifi at home, which is somewhat
> > unmetered - and access to mobile data, which often costs more (but not
> > always true).
> >
> > The API, though ".type", allows the user and app to have a conversation
> > about that: "you want me to download stuff over mobile? Its might cost
> ya,
> > but if you are ok with it...".
> >
>
> I don't really think this addresses my argument, which is not about any of
> the technical details, but is rather about whether we should adopt
> something that's clearly not very good -- which it seems clear you are
> conceding here -- just because it's been floating around a long time and
> people are tired.
>
> -Ekr
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Improving visibility of compiler warnings

[David Major]

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

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

Re: Quantum Flow Engineering Newsletter #9

[Chris Peterson]

On 2017-05-12 9:55 AM, Ehsan Akhgari wrote:

This reminded me of
https://bugzilla.mozilla.org/show_bug.cgi?id=1332680 (and
https://bugzilla.mozilla.org/show_bug.cgi?id=1332682 )

Adding -Wsuggest-final-types and -Wsuggest-final-methods and looking
at the output seems pretty low-effort to find a lot of more
opportunities for this. (Unless I'm misunderstanding things).

Plus, it benefits security!

Yes, this is indeed a good point.  Even though this will really only
have a measurable impact on performance if the functions are called in
hot code, it seems like a shame to not listen to the compiler when it
tells you I could make your code faster if you added this keyword in a
bunch of places.  :-)


Should the Mozilla Coding Style document recommend that all new classes 
use `final` unless they are designed to be derived? It would be a good 
habit even for simple classes that don't derive from a base class.


Also, Herb Sutter recommends [1] that all base classes should either 
have a public virtual destructor or protected non-virtual destructor. 
This prevents the problem where a derived class's non-virtual destructor 
doesn't get called if the object is deleted through a pointer to a base 
class.


So all classes would either:

- be a final class,
- have a public virtual destructor, or
- have a protected non-virtual dtor (possibly an empty inline dtor)


[1] http://www.gotw.ca/publications/mill18.htm
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Kris Maglione]

On Fri, May 19, 2017 at 08:44:58AM +0300, Henri Sivonen wrote:

The downsides would be that the memory for the tables wouldn't be
reclaimed if the tables aren't needed anymore (the browser can't
predict the future) and executions where any of the tables has been
created wouldn't be valgrind-clean.


If we do this, it would be nice to flush the tables when we get 
a memory-pressure event, which should at least mitigate some of 
the effects for users on memory-constrained systems.


And is there a reason ClearOnShutdown couldn't be used to deal 
with valgrind issues?


That said, can we try to get some telemetry on how often we'd 
need to build these tables, and how likely they are to be needed 
again in the same process, before we make a decision?

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

Improving visibility of compiler warnings

[Gregory Szorc]

`mach build` attempts to parse compiler warnings to a persisted "database."
You can view a list of compiler warnings post build by running `mach
warnings-list`. The intent behind this feature was to make it easier to
find and fix compiler warnings. After all, something out of sight is out of
mind.

There have been a few recent changes to increase the visibility of compiler
warnings with the expectation being that raising visibility will increase
the chances of someone addressing them. After all, a compiler warning is
either a) valid and should be fixed or b) invalid and should be ignored.
Either way, a compiler warning shouldn't exist.

First, `mach build` now prints a list of all parsed compiler warnings at
the end of the build. More details are in the commit message:
https://hg.mozilla.org/mozilla-central/rev/4abe611696ab

Second, Perfherder now records the compiler warning count as a metric. This
means we now have alerts when compiler warnings are added or removed, like
[1]. And, we can easily see graphs of compiler warnings over time, like
[2]. The compiler warnings are also surfaced in the "performance" tab of
build jobs on Treeherder, like [3].

The Perfherder alerts and tracking are informational only: nobody will be
backing you out merely because you added a compiler warning. While the
possibility of doing that now exists, I view that decision as up to the C++
module. I'm not going to advocate for it. So if you feel passionately, take
it up with them :)

Astute link clickers will note that the count metrics in CI are often noisy
- commonly fluctuating by a value of 1-2. I suspect there are race
conditions with interleaved process output and/or bugs in the compiler
warnings parser/aggregator. Since the values are currently advisory only,
I'm very much taking a "perfect is the enemy of good" attitude and have no
plans to improve the robustness.

[1] https://treeherder.mozilla.org/perf.html#/alerts?id=6700
[2] https://mzl.la/2qFN0me and https://mzl.la/2rAkWR5
[3]
https://treeherder.mozilla.org/#/jobs?repo=autoland=100509284
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Improving visibility of compiler warnings

[Kris Maglione]

I've actually been meaning to post about this, with some questions.

I definitely like that we now print warnings at the end of builds, since it 
makes them harder to ignore. But the current amount of warnings spew at the 
end of every build is problematic, especially when a build fails and I need 
to scroll up several pages to find out why.


Can we make some effort to get clean warnings output at the end of standard 
builds? A huge chunk of the warnings come from NSS and NSPR, and should be 
easily fixable. Most of the rest seem to come from vendored libraries, and 
should probably just be whitelisted if we can't get fixes upstreamed.


On Fri, May 19, 2017 at 11:44:08AM -0700, Gregory Szorc wrote:

`mach build` attempts to parse compiler warnings to a persisted "database."
You can view a list of compiler warnings post build by running `mach
warnings-list`. The intent behind this feature was to make it easier to
find and fix compiler warnings. After all, something out of sight is out of
mind.

There have been a few recent changes to increase the visibility of compiler
warnings with the expectation being that raising visibility will increase
the chances of someone addressing them. After all, a compiler warning is
either a) valid and should be fixed or b) invalid and should be ignored.
Either way, a compiler warning shouldn't exist.

First, `mach build` now prints a list of all parsed compiler warnings at
the end of the build. More details are in the commit message:
https://hg.mozilla.org/mozilla-central/rev/4abe611696ab

Second, Perfherder now records the compiler warning count as a metric. This
means we now have alerts when compiler warnings are added or removed, like
[1]. And, we can easily see graphs of compiler warnings over time, like
[2]. The compiler warnings are also surfaced in the "performance" tab of
build jobs on Treeherder, like [3].

The Perfherder alerts and tracking are informational only: nobody will be
backing you out merely because you added a compiler warning. While the
possibility of doing that now exists, I view that decision as up to the C++
module. I'm not going to advocate for it. So if you feel passionately, take
it up with them :)

Astute link clickers will note that the count metrics in CI are often noisy
- commonly fluctuating by a value of 1-2. I suspect there are race
conditions with interleaved process output and/or bugs in the compiler
warnings parser/aggregator. Since the values are currently advisory only,
I'm very much taking a "perfect is the enemy of good" attitude and have no
plans to improve the robustness.

[1] https://treeherder.mozilla.org/perf.html#/alerts?id=6700
[2] https://mzl.la/2qFN0me and https://mzl.la/2rAkWR5
[3]
https://treeherder.mozilla.org/#/jobs?repo=autoland=100509284
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


--
Kris Maglione
Senior Firefox Add-ons Engineer
Mozilla Corporation

The X server has to be the biggest program I've ever seen that doesn't
do anything for you.
--Ken Thompson

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

Re: Improving visibility of compiler warnings

[Michael Layzell]

With regard to ALLOW_COMPILER_WARNINGS, I'm strongly of the opinion that we
should be providing an option to hide all warnings from modules marked as
ALLOW_COMPILER_WARNINGS now, as they are only in "external" libraries which
most of us are not working on, and they really clog up my compiler output
and make me have to scroll up many many pages in order to see the build
errors which are actually my fault. (see this bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=1301688). Unfortunately, I
don't know enough about the build system to write a patch to do this.

On Fri, May 19, 2017 at 2:59 PM, David Major  wrote:

> 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
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org

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

[Jeff Muizelaar]

We use functions like cairo_debug_reset_static_data() on shutdown to
handle cases like this.

-Jeff

On Fri, May 19, 2017 at 1:44 AM, Henri Sivonen  wrote:
> On Tue, May 16, 2017 at 7:03 AM, Tim Guan-tin Chien
>  wrote:
>> According to Alexa top 100 Taiwan sites and quick spot checks, I can only
>> see the following two sites encoded in Big5:
>>
>> http://www.ruten.com.tw/
>> https://www.momoshop.com.tw/
>>
>> Both are shopping sites (eBay-like and Amazon-like) so you get the idea how
>> forms are used there.
>
> Thank you. It seems to me that encoder performance doesn't really
> matter for sites like these, since the number of characters one would
> enter in the search field at a time is very small.
>
>> Mike reminded me to check the Tax filing website: http://www.tax.nat.gov.tw/
>> .Yes, it's unfortunately also in Big5.
>
> I guess I'm not going to try filing taxes there for testing. :-)
>
> - -
>
> One option I've been thinking about is computing an encode
> acceleration table for JIS X 0208 on the first attempt to encode a CJK
> Unified Ideograph in any of Shift_JIS, EUC-JP or ISO-2022-JP, for GBK
> on the first attempt to encode a CJK Unified Ideograph in either GBK
> or gb18030, and for Big5 on the first attempt to encode a CJK Unified
> Ideograph in Big5.
>
> Each of the three tables would then remain allocated through to the
> termination of the process.
>
> This would have the advantage of not bloating our binary footprint
> with data that can be computed from other data in the binary while
> still making legacy Chinese and Japanese encode fast without a setup
> cost for each encoder instance.
>
> The downsides would be that the memory for the tables wouldn't be
> reclaimed if the tables aren't needed anymore (the browser can't
> predict the future) and executions where any of the tables has been
> created wouldn't be valgrind-clean. Also, in the multi-process world,
> the tables would be recomputed per-process. OTOH, if we shut down
> rendered processes from time to time, it would work as a coarse
> mechanism to reclaim the memory is case Japanese or Chinese legacy
> encode is a relatively isolated event in the user's browsing pattern.
>
> Creating a mechanism for the encoding library to become aware of
> application shutdown just in order to be valgrind-clean would be
> messy, though. (Currently, we have shutdown bugs where uconv gets used
> after we've told it can shut down. I'd really want to avoid
> re-introducing that class of bugs with encoding_rs.)
>
> Is it OK to create allocations that are intentionally never freed
> (i.e. process termination is what "frees" them)? Is valgrind's message
> suppression mechanism granular enough to suppress three allocations
> from a particular Rust crate statically linked into libxul?
>
> --
> Henri Sivonen
> hsivo...@hsivonen.fi
> https://hsivonen.fi/
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

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

[Eric Rahm]

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

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

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

-e

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

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

Re: Improving visibility of compiler warnings

[William Lachance]

On 2017-05-19 2:44 PM, Gregory Szorc wrote:

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 :)


Always happy to see people find new and unanticipated uses for Perfherder.

Should performance sheriffs file bugs when they encounter regressions?

e.g. this "alert" https://treeherder.mozilla.org/perf.html#/alerts?id=6673

has the underlying root cause: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1345368


I worry that filing a bug for every regression is going to be too much.

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

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

[Jet Villegas]

Might be good to serialize to/from disk after the first run, so only
the first process pays the compute cost?

On Thu, May 18, 2017 at 10:44 PM, Henri Sivonen  wrote:
> On Tue, May 16, 2017 at 7:03 AM, Tim Guan-tin Chien
>  wrote:
>> According to Alexa top 100 Taiwan sites and quick spot checks, I can only
>> see the following two sites encoded in Big5:
>>
>> http://www.ruten.com.tw/
>> https://www.momoshop.com.tw/
>>
>> Both are shopping sites (eBay-like and Amazon-like) so you get the idea how
>> forms are used there.
>
> Thank you. It seems to me that encoder performance doesn't really
> matter for sites like these, since the number of characters one would
> enter in the search field at a time is very small.
>
>> Mike reminded me to check the Tax filing website: http://www.tax.nat.gov.tw/
>> .Yes, it's unfortunately also in Big5.
>
> I guess I'm not going to try filing taxes there for testing. :-)
>
> - -
>
> One option I've been thinking about is computing an encode
> acceleration table for JIS X 0208 on the first attempt to encode a CJK
> Unified Ideograph in any of Shift_JIS, EUC-JP or ISO-2022-JP, for GBK
> on the first attempt to encode a CJK Unified Ideograph in either GBK
> or gb18030, and for Big5 on the first attempt to encode a CJK Unified
> Ideograph in Big5.
>
> Each of the three tables would then remain allocated through to the
> termination of the process.
>
> This would have the advantage of not bloating our binary footprint
> with data that can be computed from other data in the binary while
> still making legacy Chinese and Japanese encode fast without a setup
> cost for each encoder instance.
>
> The downsides would be that the memory for the tables wouldn't be
> reclaimed if the tables aren't needed anymore (the browser can't
> predict the future) and executions where any of the tables has been
> created wouldn't be valgrind-clean. Also, in the multi-process world,
> the tables would be recomputed per-process. OTOH, if we shut down
> rendered processes from time to time, it would work as a coarse
> mechanism to reclaim the memory is case Japanese or Chinese legacy
> encode is a relatively isolated event in the user's browsing pattern.
>
> Creating a mechanism for the encoding library to become aware of
> application shutdown just in order to be valgrind-clean would be
> messy, though. (Currently, we have shutdown bugs where uconv gets used
> after we've told it can shut down. I'd really want to avoid
> re-introducing that class of bugs with encoding_rs.)
>
> Is it OK to create allocations that are intentionally never freed
> (i.e. process termination is what "frees" them)? Is valgrind's message
> suppression mechanism granular enough to suppress three allocations
> from a particular Rust crate statically linked into libxul?
>
> --
> Henri Sivonen
> hsivo...@hsivonen.fi
> https://hsivonen.fi/
> ___
> 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

[jmaher]

It is great to see a good use for compiler warnings and alerts.  We have added 
a lot of data to perfherder and the build metrics are not covered by any 
sheriffs by default.  For these if it is clear who introduced them, we will 
comment in the bug as a note, but there are no intentions to file bugs for any 
regressions here.

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

Re: Improving visibility of compiler warnings

[Kris Maglione]

While I agree with that in general, it seems like the warnings in NSS and 
NSPR, at least, are our responsibility, and should be fixed. I notice the 
huge number of warnings scrolling by from NSS, in particular, every time I 
compile, and they make me worry.


On Fri, May 19, 2017 at 04:27:47PM -0400, Michael Layzell wrote:

With regard to ALLOW_COMPILER_WARNINGS, I'm strongly of the opinion that we
should be providing an option to hide all warnings from modules marked as
ALLOW_COMPILER_WARNINGS now, as they are only in "external" libraries which
most of us are not working on, and they really clog up my compiler output
and make me have to scroll up many many pages in order to see the build
errors which are actually my fault. (see this bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=1301688). Unfortunately, I
don't know enough about the build system to write a patch to do this.

On Fri, May 19, 2017 at 2:59 PM, David Major  wrote:


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

Re: Improving visibility of compiler warnings

[Bill McCloskey]

I strongly agree with you, Michael! Especially now that I'm using IceCC, I
pretty much always have to use search-and-replace to find compiler errors.
It's a pointless drain on productivity.

On Fri, May 19, 2017 at 1:27 PM, Michael Layzell 
wrote:

> With regard to ALLOW_COMPILER_WARNINGS, I'm strongly of the opinion that we
> should be providing an option to hide all warnings from modules marked as
> ALLOW_COMPILER_WARNINGS now, as they are only in "external" libraries which
> most of us are not working on, and they really clog up my compiler output
> and make me have to scroll up many many pages in order to see the build
> errors which are actually my fault. (see this bug:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1301688). Unfortunately, I
> don't know enough about the build system to write a patch to do this.
>
> On Fri, May 19, 2017 at 2:59 PM, David Major  wrote:
>
> > 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 

Re: Improving visibility of compiler warnings

[Gregory Szorc]

On Fri, May 19, 2017 at 1:27 PM, Michael Layzell 
wrote:

> With regard to ALLOW_COMPILER_WARNINGS, I'm strongly of the opinion that we
> should be providing an option to hide all warnings from modules marked as
> ALLOW_COMPILER_WARNINGS now, as they are only in "external" libraries which
> most of us are not working on, and they really clog up my compiler output
> and make me have to scroll up many many pages in order to see the build
> errors which are actually my fault. (see this bug:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1301688). Unfortunately, I
> don't know enough about the build system to write a patch to do this.
>

I like to teach others how to fish:

https://dxr.mozilla.org/mozilla-central/search?q=path%3Amoz.build+C=false

Code to call into external build systems is typically in config/external/.
See e.g. config/external/icu/defs.mozbuild for where compiler flags for ICU
are defined.



>
> On Fri, May 19, 2017 at 2:59 PM, David Major  wrote:
>
> > 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
> 

Re: Changing our thread APIs for Quantum DOM scheduling

[Chris Peterson]

The Quantum DOM doc says only content processes will get cooperative 
threading. How will cooperative threading work with multiple content 
processes (e10s-multi)? Will there be inter-process scheduling? For 
example, if content process #1 has one or more foreground tabs (from 
multiple windows) and content process #2 has only background tabs, will 
content process #2 yield to content process #1? Or will content process 
#2 continue to run all of its background tabs because it doesn't know of 
any foreground tabs to prioritize?



On 2017-05-18 4:38 PM, Bill McCloskey wrote:

Hi everyone,

One of the challenges of the Quantum DOM project is that we will soon have
multiple "main" threads [1]. These will be real OS threads, but only one of
them will be allowed to run code at any given time. We will switch between
them at well-defined points (currently just the JS interrupt callback).
This cooperative scheduling will make it much easier to keep our global
state consistent. However, having multiple "main" threads is likely to
cause confusion.

To simplify things, we considered trying to make these multiple threads
"look" like a single main thread at the API level, but it's really hard to
hide something like that. So, instead, we're going to be transitioning to
APIs that try to avoid exposing threads at all. This post will summarize
that effort. You can find more details in this Google doc:

https://docs.google.com/document/d/1MZhF1zB5_dk12WRiq4bpmNZUJWmsIt9OTpFUWAlmMyY/edit?usp=sharing

[Note: I'd like this thread (and the Google doc) to be for discussing
threading APIs. If you have more general questions about the project,
please contact me personally.]

The main API change is that we're going to make it a lot harder to get hold
of an nsIThread for the main thread. Instead, we want people to work with
event targets (nsIEventTarget). The advantage of event targets is that all
the main threads will share a single event loop, and therefore a single
nsIEventTarget. So code that once expected a single main thread can now
expect a single main event target.

The functions NS_GetMainThread, NS_GetCurrentThread, and
do_Get{Main,Current}Thread will be deprecated. In their place, we'll
provide mozilla::Get{Main,Current}ThreadEventTarget. These functions will
return an event target instead of a thread.

More details:

NS_IsMainThread: This function will remain pretty much the same. It will
return true on any one of the main threads and false elsewhere.

Dispatching runnables: NS_DispatchToMainThread will still work, and you
will still be able to dispatch using Get{Main,Current}ThreadEventTarget.

From JS, we want people to use Services.tm.dispatchToMainThread.


Thread-safety assertions: Code that used PR_GetCurrentThread for thread
safety assertions will be converted to use NS_ASSERT_OWNINGTHREAD, which
will allow code from different main threads to touch the same object.
PR_GetCurrentThread will be deprecated. If you really want to get the
current PRThread*, you can use GetCurrentPhysicalThread, which will return
a different value for each main thread.

Code that uses NS_GetCurrentThread for thread safety assertions will be
converted to use nsIEventTarget::IsOnCurrentThread. The main thread event
target will return true from IsOnCurrentThread if you're on any of the main
threads.

Nested event loop spinning: In the future, we want people to use
SpinEventLoopUntil to spin a nested event loop. It will do the right thing
when called on any of the main threads. We'll provide a similar facility to
JS consumers.

Bugs:

Bug 1359903 converted some of our PR_GetCurrentThread assertions to use
NS_ASSERT_OWNINGTHREAD.

Bug 1361561 added GetCurrentPhysicalThread and GetCurrentVirtualThread.
These functions both return a PRThread*. The latter one returns the same
value for all the main threads. It should only be used for assertion
checking.

Bug 1361164 will add the Get{Current,Main}ThreadEventTarget functions.

Bug 1365096 is a metabug to convert all uses of NS_Get{Current,Main}Thread
to use event targets instead. Bug 1365097 is the bug for converting DOM
code.

[1] https://billmccloskey.wordpress.com/2016/10/27/mozillas-quantum-project/



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