Re: PSA: Automated code analysis now also in Phabricator

[Jan Keromnes]

> It might be better to have some indicator that the analysis *did* run,
> because otherwise, you can't tell if you should wait longer for the
> result or whether your code is clean.

Originally, our bot *did* publish a comment when analysis didn't find any
defects in a patch, but this was causing a lot of bugmail noise:
https://groups.google.com/d/msg/mozilla.dev.platform/TFfjCRdGz_E/uCQx4XHZCAAJ

So we decided that it was better to publish comments only when *some*
defects were found.

However, we're now considering adding a custom Phabricator build status
indicator to show when our analysis is running, or if it has succeeded or
failed:

> - Integrate more deeply with Phabricator, e.g. by reporting a build
status for our analysis

This would be a lighter / less chatty integration than publishing comments.

Additionally, our analysis generally finishes in under 8 minutes, so if you
haven't heard from the bot in a while it means your patch is probably fine:
https://p.datadoghq.com/sb/NsBIKn-240d11775d25dce0ef2c47d4b7ce0ae0

Jan

On Tue, Jul 17, 2018 at 11:10 PM Mike Hommey  wrote:

> On Tue, Jul 17, 2018 at 05:06:07PM +0200, Jan Keromnes wrote:
> > Thanks all for the enthusiasm, we're also excited about what this can do
> > for us.
> >
> > > When did this become active?
> >
> > Last year on MozReview, yesterday on Phabricator.
> >
> > > Can existing diff be forced to be scanned if they weren’t before?
> >
> > The easiest way to force a re-scan is to re-upload the patch (e.g. after
> > rebasing it).
> >
> > Note that the bot doesn't publish anything if no defect was detected. A
> > complete analysis generally takes a few minutes.
>
> It might be better to have some indicator that the analysis *did* run,
> because otherwise, you can't tell if you should wait longer for the
> result or whether your code is clean.
>
> Mike
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: PSA: Major preference service architecture changes inbound

[Nicholas Nethercote]

On Wed, Jul 18, 2018 at 1:57 AM, Kris Maglione 
wrote:

>
> While we're thinking about the prefs service, is there any possibility we
>> could enable off-main-thread access to preferences?
>>
>
> I think the chances of that are pretty close to 0, but I'll defer to Nick.
>

I agree, for the reasons that Kris mentioned.



> I need another thread to be able to query an extensible set of pref names
> that are not fully known at compile time.
>

This is a good example of how prefs is a far more general mechanism than I
would like, leading to all manner of use and abuse. "All I want is a
key-value store, with fast multi-threaded access, where the keys aren't
known ahead of time."

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

Intent to unship: Web animations composite modes on keyframes

[Brian Birtles]

As per my recent "Intent to ship: Web Animations core interfaces"[1], we do
not intend to ship Animation composite modes until various spec work has
been completed.

However, when we implemented this feature, although we implemented it
behind a pref, we failed to turn off all the ways of accessing it.

Composite modes may be set in two ways:

1. Globally

  e.g. elem.animate([{ opacity: 0 }, { opacity: 1 }], { duration: 2000,
composite: 'add' });

2. Per-keyframe:

  e.g. elem.animate([{ opacity: 0, composite: 'add' }, { opacity: 1 }], {
duration: 2000 });

It turns out we successfully pref'ed off the feature when set globally (1,
from above) and even have automated tests to cover this, but we did not
pref off the feature when setting it per-keyframe (2).

No other UA is shipping this feature yet. As a result we don't have any
telemetry data regarding usage. However, given that no other UA ships this,
that it is not well known, and that setting the value on individual
keyframes is generally less convenient than setting it globally, I expect
this is not used at all outside of toy API demos.

Furthermore, by unshipping this Firefox will simply match the behavior of
current Chrome (that is, any animation using the feature would still run,
but would not show the additive behavior).

I think it is best to unship this now until it is properly specified and
then ship both modes together. As a result, in bug 1471814, which reached
m-c just moments ago, I have gone ahead and guarded this feature behind the
dom.animations-api.compositing.enabled pref (enabled in nightly only).

Please let me know if you have any concerns.

Best regards,

Brian

[1] https://groups.google.com/forum/#!topic/mozilla.dev.platform/fcFctnUjs7A
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: PSA: Major preference service architecture changes inbound

[Jeff Gilbert]

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

On Tue, Jul 17, 2018 at 8:57 AM, Kris Maglione  wrote:
> On Tue, Jul 17, 2018 at 02:06:48PM +0100, Jonathan Kew wrote:
>>
>> On 13/07/2018 21:37, Kris Maglione wrote:
>>>
>>> tl;dr: A major change to the architecture preference service has just
>>> landed, so please be on the lookout for regressions.
>>>
>>> We've been working for the last few weeks on rearchitecting the
>>> preference service to work better in our current and future multi-process
>>> configurations, and those changes have just landed in bug 1471025.
>>
>>
>> Looks like a great step forward!
>>
>> While we're thinking about the prefs service, is there any possibility we
>> could enable off-main-thread access to preferences?
>
>
> I think the chances of that are pretty close to 0, but I'll defer to Nick.
>
> We definitely can't afford the locking overhead—preference look-ups already
> show up in profiles without it. And even the current limited exception that
> we grant Stylo while it has the main thread blocked causes problems (bug
> 1474789), since it makes it impossible to update statistics for those reads,
> or switch to Robin Hood hashing (which would make our hash tables much
> smaller and more efficient, but requires read operations to be able to move
> entries).
>
>> I am aware that in simple cases, this can be achieved via the
>> StaticPrefsList; by defining a VARCACHE_PREF there, I can read its value
>> from other threads. But this doesn't help in my use case, where I need
>> another thread to be able to query an extensible set of pref names that are
>> not fully known at compile time.
>>
>> Currently, it looks like to do this, I'll have to iterate over the
>> relevant prefs branch(es) ahead of time (on the main thread) and copy all
>> the entries to some other place that is then available to my worker threads.
>> For my use case, at least, the other threads only need read access;
>> modifying prefs could still be limited to the main thread.
>
>
> That's probably your best option, yeah. Although I will say that those kinds
> of extensible preference sets aren't great for performance or memory usage,
> so switching to some other model might be better.
>
>> Possible? Or would the overhead of locking be too crippling?
>
>
> The latter, I'm afraid.
>
> ___
> 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

Intent to ship: Web Animations core interfaces

[Brian Birtles]

As of 19 July or thereabouts, I intend to turn on the Web Animations core
interfaces by default on all platforms.

It has been developed behind the dom.animations-api.core.enabled preference.

What is already shipping?

* Animation interface with playback methods
* Animation finish / cancel events
* Element.animate() method

What does this intent add to that?

* Animation.ready / Animation.finished Promises
* Animation.effect member
* KeyframeEffect and AnimationEffect interfaces

What is *not* included in this intent?

* Animations with implicit 0% / 100% keyframes
  (dom.animations-api.implicit-keyframes.enabled)
  - This feature can easily lead to excessive memory usage which we
should address before shipping this.
* Animation composite modes (additive animation)
  (dom.animations-api.compositing.enabled)
  - We need to properly specify _how_ each property type adds together
before shipping this. I plan to work with fantasai on specifying
this in September.
* Animation timelines
  (dom.animations-api.timelines.enabled)
  - Although it might be useful to ship a read-only version of this
soon, we should clarify how AnimationWorklet will integrate with
timelines before shipping a writable version of this API.
* Element.getAnimations() / Document.getAnimations()
  (dom.animations-api.getAnimations.enabled)
  - How CSS animations / transitions are reflected in the API needs to
be specified before we can ship this. The spec for the
CSSPseudoElement interface should also be more stable.

Bug to turn on by default: https://bugzilla.mozilla.org/
show_bug.cgi?id=1476158

This feature was previously discussed in this "intent to implement" thread:
https://groups.google.com/d/topic/mozilla.dev.platform/
3EXBgp26rJs/discussion

What is the status of other UAs?

* Chrome:
  - Already ships the same subset as we do
  - Implements most of the additional features covered by this intent
guarded behind the "Experimental Web Platform features" flag.
The only notable exceptions are making KeyframeEffect.target
writeable and some updates to reflect recent changes to timing
interfaces.
* Safari:
  - Implements most of the features covered by this intent in the latest
Technology Preview. The main cause of wpt test failures is that
Safari has not yet updated to some recent changes to timing
interfaces.
* Edge:
  - Roadmap Priority: Medium[1]

Best regards,

Brian

[1] https://developer.microsoft.com/en-us/microsoft-edge/platform/status/
webanimationsjavascriptapi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: PSA: Automated code analysis now also in Phabricator

[Mike Hommey]

On Tue, Jul 17, 2018 at 05:06:07PM +0200, Jan Keromnes wrote:
> Thanks all for the enthusiasm, we're also excited about what this can do
> for us.
> 
> > When did this become active?
> 
> Last year on MozReview, yesterday on Phabricator.
> 
> > Can existing diff be forced to be scanned if they weren’t before?
> 
> The easiest way to force a re-scan is to re-upload the patch (e.g. after
> rebasing it).
> 
> Note that the bot doesn't publish anything if no defect was detected. A
> complete analysis generally takes a few minutes.

It might be better to have some indicator that the analysis *did* run,
because otherwise, you can't tell if you should wait longer for the
result or whether your code is clean.

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

Re: PSA: Re-run old (non-syntax) try pushes with |mach try again|

[James Graham]

On 17/07/2018 21:16, Nicholas Alexander wrote:

Ahal,

On Tue, Jul 17, 2018 at 11:55 AM, Andrew Halberstadt > wrote:


While |mach try fuzzy| is generally a better experience than try
syntax, there are a few cases where it can be annoying. One
common case was when you selected a bunch of tasks in the
interface and pushed. Then at a later date you wanted to push
the exact same set of tasks again. This used to be a really poor
experience as you needed to re-select all the same tasks
manually.

As of now, you can use |mach try again| instead. The general
workflow is:

This is awesome, thank you for building it!

Can it be extended to "named pushes"?  That is, right now I use my shell 
history to do `mach try fuzzy -q "'build-android | 'robocop", but nobody 
else will find that without me telling them, and it won't be 
automatically updated when robocop gets renamed.  That is, if I could 
`mach try fuzzy --named android-tier1` or something I could save myself 
some manual command editing and teach other people what a green try run 
means in my area.


./mach try fuzzy --save android-tier1 -q "'build-android | 'robocop"

And then run with

./mach try fuzzy --preset android-tier1

I think that's what you want? There isn't a way to share it or anything, 
but it works well for the use case of "I make the same set of try pushes 
repeatedly over many patches".

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

Re: PSA: Re-run old (non-syntax) try pushes with |mach try again|

[Nicholas Alexander]

Ahal,

On Tue, Jul 17, 2018 at 11:55 AM, Andrew Halberstadt 
wrote:

> While |mach try fuzzy| is generally a better experience than try
> syntax, there are a few cases where it can be annoying. One
> common case was when you selected a bunch of tasks in the
> interface and pushed. Then at a later date you wanted to push
> the exact same set of tasks again. This used to be a really poor
> experience as you needed to re-select all the same tasks
> manually.
>
> As of now, you can use |mach try again| instead. The general
> workflow is:
>

This is awesome, thank you for building it!

Can it be extended to "named pushes"?  That is, right now I use my shell
history to do `mach try fuzzy -q "'build-android | 'robocop", but nobody
else will find that without me telling them, and it won't be automatically
updated when robocop gets renamed.  That is, if I could `mach try fuzzy
--named android-tier1` or something I could save myself some manual command
editing and teach other people what a green try run means in my area.

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

PSA: Re-run old (non-syntax) try pushes with |mach try again|

[Andrew Halberstadt]

While |mach try fuzzy| is generally a better experience than try
syntax, there are a few cases where it can be annoying. One
common case was when you selected a bunch of tasks in the
interface and pushed. Then at a later date you wanted to push
the exact same set of tasks again. This used to be a really poor
experience as you needed to re-select all the same tasks
manually.

As of now, you can use |mach try again| instead. The general
workflow is:

 $ ./mach try fuzzy

...


$ ./mach try again


*More Details*

Whenever you push to try with a `try_task_config.json` (aka
anything but try syntax), that config will be saved in a history
file (in ~/.mozbuild/history/try_task_configs.json). You can view
your history keyed by commit message by running:

$ ./mach try again --list

If you wish to re-push an older `try_task_config.json`, you can
use:

$ ./mach try again --index 

Where  is the number displayed by --list. By default the
last 10 pushes will be saved in history, but this can be
configured by setting:

[try]
maxhistory=

in your ~/.mozbuild/machrc.


*Caveats*

As mentioned earlier, pushes with try syntax aren't saved in
history as they don't use `try_task_config.json` files. When/if
bug 1400295 lands, then pushes generated with try syntax
will start working.

The full taskgraph might change between the time you
originally ran the push and the time you re-push with |mach try
again|. If this happens, it's possible the tasks you previously
scheduled were renamed (or removed) and the decision task
will fail.

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

PSA: [implicit_jscontext] XPIDL methods/attributes can now be implemented in JS

[Jan de Mooij]

Hi all,

It's now possible to have JS implementations of an XPIDL method/attribute
marked as [implicit_jscontext]. This used to throw an exception because the
xptcall stubs didn't support it. Bug 1475699 [0] fixes the stubs to skip
the implicit context argument.

This is useful when you have both JS and C++ implementations of an IDL
method/attribute and you want the C++ implementations to have the
JSContext* argument.

Jan

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

Re: PSA: Major preference service architecture changes inbound

[Kris Maglione]

On Tue, Jul 17, 2018 at 02:06:48PM +0100, Jonathan Kew wrote:

On 13/07/2018 21:37, Kris Maglione wrote:
tl;dr: A major change to the architecture preference service has 
just landed, so please be on the lookout for regressions.


We've been working for the last few weeks on rearchitecting the 
preference service to work better in our current and future 
multi-process configurations, and those changes have just landed in 
bug 1471025.


Looks like a great step forward!

While we're thinking about the prefs service, is there any possibility 
we could enable off-main-thread access to preferences?


I think the chances of that are pretty close to 0, but I'll 
defer to Nick.


We definitely can't afford the locking overhead—preference 
look-ups already show up in profiles without it. And even the 
current limited exception that we grant Stylo while it has the 
main thread blocked causes problems (bug 1474789), since it 
makes it impossible to update statistics for those reads, or 
switch to Robin Hood hashing (which would make our hash tables 
much smaller and more efficient, but requires read operations to 
be able to move entries).


I am aware that in simple cases, this can be achieved via the 
StaticPrefsList; by defining a VARCACHE_PREF there, I can read its 
value from other threads. But this doesn't help in my use case, where 
I need another thread to be able to query an extensible set of pref 
names that are not fully known at compile time.


Currently, it looks like to do this, I'll have to iterate over the 
relevant prefs branch(es) ahead of time (on the main thread) and copy 
all the entries to some other place that is then available to my 
worker threads. For my use case, at least, the other threads only need 
read access; modifying prefs could still be limited to the main 
thread.


That's probably your best option, yeah. Although I will say that 
those kinds of extensible preference sets aren't great for 
performance or memory usage, so switching to some other model 
might be better.



Possible? Or would the overhead of locking be too crippling?


The latter, I'm afraid.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: PSA: Automated code analysis now also in Phabricator

[Jan Keromnes]

Thanks all for the enthusiasm, we're also excited about what this can do
for us.

> When did this become active?

Last year on MozReview, yesterday on Phabricator.

> Can existing diff be forced to be scanned if they weren’t before?

The easiest way to force a re-scan is to re-upload the patch (e.g. after
rebasing it).

Note that the bot doesn't publish anything if no defect was detected. A
complete analysis generally takes a few minutes.

Jan


Am 17.07.2018 16:55 schrieb "Jean-Yves Avenard" :

Hi


On 17 Jul 2018, at 3:22 pm, Jan Keromnes  wrote:

TL;DR -- “reviewbot” is now enabled in Phabricator. It reports potential
defects in pending patches for Firefox.

Last year, we announced Code Review Bot (“reviewbot”, née “clangbot”), a
Taskcluster bot that analyzes every patch submitted to MozReview, in order
to automatically detect and report code defects *before* they land in
Nightly:

https://groups.google.com/d/msg/mozilla.dev.platform/TFfjCRdGz_E/8leqTqvBCAAJ

Developer feedback has been very positive, and the bot has caught many
defects, thus improving the quality of Firefox.


This is great … Thank you

When did this become active?

Can existing diff be forced to be scanned if they weren’t before?

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

Re: PSA: Automated code analysis now also in Phabricator

[Jean-Yves Avenard]

Hi

> On 17 Jul 2018, at 3:22 pm, Jan Keromnes  wrote:
> 
> TL;DR -- “reviewbot” is now enabled in Phabricator. It reports potential 
> defects in pending patches for Firefox.
> 
> Last year, we announced Code Review Bot (“reviewbot”, née “clangbot”), a 
> Taskcluster bot that analyzes every patch submitted to MozReview, in order to 
> automatically detect and report code defects *before* they land in Nightly:
> 
> https://groups.google.com/d/msg/mozilla.dev.platform/TFfjCRdGz_E/8leqTqvBCAAJ 
> 
> 
> Developer feedback has been very positive, and the bot has caught many 
> defects, thus improving the quality of Firefox.

This is great … Thank you

When did this become active?

Can existing diff be forced to be scanned if they weren’t before?

JY

smime.p7s
Description: S/MIME cryptographic signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: PSA: Automated code analysis now also in Phabricator

[Ted Mielczarek]

On Tue, Jul 17, 2018, at 9:22 AM, Jan Keromnes wrote:
> TL;DR -- “reviewbot” is now enabled in Phabricator. It reports potential
> defects in pending patches for Firefox.

Great work! This sounds super useful!

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

Re: PSA: Automated code analysis now also in Phabricator

[Eric Rescorla]

This is amazing and looks super-useful. Really looking forward to seeing
what else we can add in this area!

-Ekr


On Tue, Jul 17, 2018 at 6:22 AM, Jan Keromnes  wrote:

> TL;DR -- “reviewbot” is now enabled in Phabricator. It reports potential
> defects in pending patches for Firefox.
>
> Last year, we announced Code Review Bot (“reviewbot”, née “clangbot”), a
> Taskcluster bot that analyzes every patch submitted to MozReview, in order
> to automatically detect and report code defects *before* they land in
> Nightly:
>
> https://groups.google.com/d/msg/mozilla.dev.platform/TFfjCRd
> Gz_E/8leqTqvBCAAJ
>
> Developer feedback has been very positive, and the bot has caught many
> defects, thus improving the quality of Firefox.
>
> Today, we’re happy to announce that reviewbot analyzes every patch
> submitted to Phabricator as well.
>
> Here is an example of an automated review in Phabricator:
> https://phabricator.services.mozilla.com/D2120
>
> Additionally, we’ve made a number of improvements to this bot over the past
> months. Notably:
> - Enabled several clang-tidy checks to catch more C/C++ defects (e.g.
> performance and security issues)
> - Integrated Mozlint in order to catch JS/Python/wpt defects as well
> - Fixed several bugs, like the lowercase code snippets in comments
> - We’re now detecting up to 5x more defects in some patches
>
> Please report any bugs with the bot here: https://bit.ly/2tb8Qk3
>
> As for next steps, we’re currently discussing a few ideas for the project’s
> future, including:
> - Catch more bugs by comparing defects before & after a patch is applied
> (currently, we report defects located on lines that are directly modified
> by the patch)
> - Evaluate which defect types are generally being fixed or ignored
> - Evaluate analyzing Rust code with rust-clippy
> - Help with coding style by leveraging clang-format
> - Integrate more deeply with Phabricator, e.g. by reporting a build status
> for our analysis
> - Integrate our analysis with Try, in order to unify our various CI and
> code analysis tools
>
> Many thanks to everyone who helped make this a reality: Bastien, who did
> most of the implementation and bug fixing, Marco, Andi, Calixte, Sylvestre,
> Ahal, the Release Management Analysis team and the Engineering Workflow
> team.
>
> Jan
> ___
> 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: Automated code analysis now also in Phabricator

[Andrew Halberstadt]

Congrats, thanks to everyone involved for getting this working!

I really like the idea of comparing errors with and without the patch,
this would let us lint code where linting isn't explicitly enabled in
mach lint/CI. One caveat to doing is that the review bot would need
to make it very clear which issues are "optional" (aka can safely be
ignored) and which ones are "required" (aka would cause a backout
if they don't get fixed).

I have some ideas of making this distinction directly in mozlint by
treating all "warnings" as optional fixes and all "errors" as required
fixes. See https://bugzilla.mozilla.org/show_bug.cgi?id=1460856

Andrew

On Tue, Jul 17, 2018 at 9:22 AM Jan Keromnes  wrote:

> TL;DR -- “reviewbot” is now enabled in Phabricator. It reports potential
> defects in pending patches for Firefox.
>
> Last year, we announced Code Review Bot (“reviewbot”, née “clangbot”), a
> Taskcluster bot that analyzes every patch submitted to MozReview, in order
> to automatically detect and report code defects *before* they land in
> Nightly:
>
>
> https://groups.google.com/d/msg/mozilla.dev.platform/TFfjCRdGz_E/8leqTqvBCAAJ
>
> Developer feedback has been very positive, and the bot has caught many
> defects, thus improving the quality of Firefox.
>
> Today, we’re happy to announce that reviewbot analyzes every patch
> submitted to Phabricator as well.
>
> Here is an example of an automated review in Phabricator:
> https://phabricator.services.mozilla.com/D2120
>
> Additionally, we’ve made a number of improvements to this bot over the
> past months. Notably:
> - Enabled several clang-tidy checks to catch more C/C++ defects (e.g.
> performance and security issues)
> - Integrated Mozlint in order to catch JS/Python/wpt defects as well
> - Fixed several bugs, like the lowercase code snippets in comments
> - We’re now detecting up to 5x more defects in some patches
>
> Please report any bugs with the bot here: https://bit.ly/2tb8Qk3
>
> As for next steps, we’re currently discussing a few ideas for the
> project’s future, including:
> - Catch more bugs by comparing defects before & after a patch is applied
> (currently, we report defects located on lines that are directly modified
> by the patch)
> - Evaluate which defect types are generally being fixed or ignored
> - Evaluate analyzing Rust code with rust-clippy
> - Help with coding style by leveraging clang-format
> - Integrate more deeply with Phabricator, e.g. by reporting a build status
> for our analysis
> - Integrate our analysis with Try, in order to unify our various CI and
> code analysis tools
>
> Many thanks to everyone who helped make this a reality: Bastien, who did
> most of the implementation and bug fixing, Marco, Andi, Calixte, Sylvestre,
> Ahal, the Release Management Analysis team and the Engineering Workflow
> team.
>
> Jan
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

PSA: Automated code analysis now also in Phabricator

[Jan Keromnes]

TL;DR -- “reviewbot” is now enabled in Phabricator. It reports potential
defects in pending patches for Firefox.

Last year, we announced Code Review Bot (“reviewbot”, née “clangbot”), a
Taskcluster bot that analyzes every patch submitted to MozReview, in order
to automatically detect and report code defects *before* they land in
Nightly:

https://groups.google.com/d/msg/mozilla.dev.platform/TFfjCRdGz_E/8leqTqvBCAAJ

Developer feedback has been very positive, and the bot has caught many
defects, thus improving the quality of Firefox.

Today, we’re happy to announce that reviewbot analyzes every patch
submitted to Phabricator as well.

Here is an example of an automated review in Phabricator:
https://phabricator.services.mozilla.com/D2120

Additionally, we’ve made a number of improvements to this bot over the past
months. Notably:
- Enabled several clang-tidy checks to catch more C/C++ defects (e.g.
performance and security issues)
- Integrated Mozlint in order to catch JS/Python/wpt defects as well
- Fixed several bugs, like the lowercase code snippets in comments
- We’re now detecting up to 5x more defects in some patches

Please report any bugs with the bot here: https://bit.ly/2tb8Qk3

As for next steps, we’re currently discussing a few ideas for the project’s
future, including:
- Catch more bugs by comparing defects before & after a patch is applied
(currently, we report defects located on lines that are directly modified
by the patch)
- Evaluate which defect types are generally being fixed or ignored
- Evaluate analyzing Rust code with rust-clippy
- Help with coding style by leveraging clang-format
- Integrate more deeply with Phabricator, e.g. by reporting a build status
for our analysis
- Integrate our analysis with Try, in order to unify our various CI and
code analysis tools

Many thanks to everyone who helped make this a reality: Bastien, who did
most of the implementation and bug fixing, Marco, Andi, Calixte, Sylvestre,
Ahal, the Release Management Analysis team and the Engineering Workflow
team.

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

Re: PSA: Major preference service architecture changes inbound

[Jonathan Kew]

On 13/07/2018 21:37, Kris Maglione wrote:
tl;dr: A major change to the architecture preference service has just 
landed, so please be on the lookout for regressions.


We've been working for the last few weeks on rearchitecting the 
preference service to work better in our current and future 
multi-process configurations, and those changes have just landed in bug 
1471025.


Looks like a great step forward!

While we're thinking about the prefs service, is there any possibility 
we could enable off-main-thread access to preferences?


I am aware that in simple cases, this can be achieved via the 
StaticPrefsList; by defining a VARCACHE_PREF there, I can read its value 
from other threads. But this doesn't help in my use case, where I need 
another thread to be able to query an extensible set of pref names that 
are not fully known at compile time.


Currently, it looks like to do this, I'll have to iterate over the 
relevant prefs branch(es) ahead of time (on the main thread) and copy 
all the entries to some other place that is then available to my worker 
threads. For my use case, at least, the other threads only need read 
access; modifying prefs could still be limited to the main thread.


Possible? Or would the overhead of locking be too crippling?

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