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

2018-07-13 Thread Boris Zbarsky

On 7/13/18 5:22 PM, gsquel...@mozilla.com wrote:

E.g., could I instrument one class, so that every allocation would be tracked 
automatically, and I'd get nice stats at the end?


You mean apart from just having a memory reporter for it?


Including wasted space because of larger allocation blocks?


Memory reporters using mallocSizeOf include that space, yes.


Could I even run what-if scenarios, where I could instrument a class and 
extract its current size but also provide an alternate size (based on what I 
think I could make it shrink), and in the end I'll know how much I could save 
overall?


You could hack the relevant memory reporter, sure.


Do we have Try tests that simulate real-world usage, so we could collect 
memory-usage data that's relevant to our users, but also reproducible?


See the "awsy-10s" test suite, which sort of aims to do that.


Should there be some kind of Talos-like CI tests that focus on memory usage, so 
we'd get some warning if a particular patch suddenly eats too much memory?


This is what awsy-e10s aims to do, yes.

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


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

2018-07-13 Thread gsquelart
On Wednesday, July 11, 2018 at 4:19:15 AM UTC+10, Kris Maglione wrote:
> [...]
> Essentially what this means, though, is that if we identify an area of 
> overhead that's 50KB[3] or larger that can be eliminated, it *has* to be 
> eliminated. There just aren't that many large chunks to remove. They all need 
> to go. And if an area of code has a dozen 5KB chunks that can be eliminated, 
> maybe they don't all have to go, but at least half of them do. The more the 
> better.

Some questions: -- Sorry if some of this is already common knowledge or has 
been discussed.

Are there tools available, that could easily track memory usage of specific 
things?
E.g., could I instrument one class, so that every allocation would be tracked 
automatically, and I'd get nice stats at the end?
Including wasted space because of larger allocation blocks?

Could I even run what-if scenarios, where I could instrument a class and 
extract its current size but also provide an alternate size (based on what I 
think I could make it shrink), and in the end I'll know how much I could save 
overall?

Do we have Try tests that simulate real-world usage, so we could collect 
memory-usage data that's relevant to our users, but also reproducible?

Should there be some kind of Talos-like CI tests that focus on memory usage, so 
we'd get some warning if a particular patch suddenly eats too much memory?
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: XUL Overlays Removed

2018-07-13 Thread Brian Grinstead



> On Jul 13, 2018, at 3:05 PM, Frank-Rainer Grahl  wrote:
> 
> > This is just one piece of the broader XUL removal effort, but it does
> > highlight that things can be simpler in a post-XUL world.
> 
> Well I agree that cleaning up overlay usage was overdue. Otherwise the simple 
> post XUL world world is just dumb. Removing things without a functional 
> replacement and putting in spaghetthi code seems to be the current mantra. 
> Preprocessing with include files is even worse.

The preprocessor was a perfectly good replacement for the way we used overlays 
used in the Firefox UI - it makes it easier to look at a document and figure 
out which resources are going to be loaded into it. Work to simplify our chrome 
documents (including reducing preprocessing) can now be done from a foundation 
where we know which documents will be affected by changes.

Brian

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


Re: XUL Overlays Removed

2018-07-13 Thread Frank-Rainer Grahl

> This is just one piece of the broader XUL removal effort, but it does
> highlight that things can be simpler in a post-XUL world.

Well I agree that cleaning up overlay usage was overdue. Otherwise the simple 
post XUL world world is just dumb. Removing things without a functional 
replacement and putting in spaghetthi code seems to be the current mantra. 
Preprocessing with include files is even worse.


FRG


Brendan Dahl wrote:

This is hopefully the last thing you’ll ever hear about XUL overlays as
they have now been completely removed from Firefox[1]. For those unfamiliar
with overlays, they provided a way to merge two XUL documents and were
mainly used by legacy extensions and in several places within the Firefox
UI. While overlays served a purpose, they were removed since we no longer
support legacy extensions and they added unneeded complexity to Firefox.


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


Re: XUL Overlays Removed

2018-07-13 Thread Kris Maglione

On Fri, Jul 13, 2018 at 02:10:54PM -0700, Brendan Dahl wrote:

This is hopefully the last thing you’ll ever hear about XUL overlays as
they have now been completely removed from Firefox[1].


\o/ Thank you to everyone who was involved in this!


Removing overlays cut around 3.5K lines of code from Firefox and in my
opinion made understanding which resources get loaded into which documents
easier to reason about


I completely agree. I can't count the number of times I've made 
a change in one place that seemed obviously sound, only to have 
it break on try because some completely unrelated piece of code 
loads a special overlay into the window only on OS-X.

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


XUL Overlays Removed

2018-07-13 Thread Brendan Dahl
This is hopefully the last thing you’ll ever hear about XUL overlays as
they have now been completely removed from Firefox[1]. For those unfamiliar
with overlays, they provided a way to merge two XUL documents and were
mainly used by legacy extensions and in several places within the Firefox
UI. While overlays served a purpose, they were removed since we no longer
support legacy extensions and they added unneeded complexity to Firefox.

Removing overlays cut around 3.5K lines of code from Firefox and in my
opinion made understanding which resources get loaded into which documents
easier to reason about (see one example of a before[2] and after[3] below).
This is just one piece of the broader XUL removal effort, but it does
highlight that things can be simpler in a post-XUL world.

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

[2] https://bug1441378.bmoattachments.org/attachment.cgi?id=8954951

[3] https://bug1441378.bmoattachments.org/attachment.cgi?id=8954952

Key:

Arrow direction: where elements go from -> to.

Red: MacOS only

Green: Non-MacOS

Blue: where the  element went
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


PSA: Major preference service architecture changes inbound

2018-07-13 Thread Kris Maglione
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.


Our preference database tends to be very large, even without any user values. 
It also needs to be available in every process. Until now, that's meant 
complete separate copies of the hash table, name strings, and value strings in 
each process, along with separate initialization in each content process, and 
a lot of IPC overhead to keep the databases in sync.


After bug 1471025, the database is split into two sections: a snapshot of the 
initial state of the database, which is stored in a read-only shared memory 
region and shared by all processes, and a dynamic hash table of changes on top 
of that snapshot, of which each process has its own. This approach 
significantly decreases memory, IPC, and content process initialization 
overhead. It also decreases the complexity of certain cross-process 
synchronization logic.


But it adds complexity in other areas, and represents one of the largest 
changes to the workings of the preference service since its creation. So 
please be on the lookout for regressions that look related to preference 
handling. If you spot any, please file bugs blocking https://bugzil.la/1471025.


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


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

2018-07-13 Thread Kris Maglione

On Fri, Jul 13, 2018 at 11:14:24AM -0400, Randell Jesup wrote:

Hash tables are a big issue.  There are a lot of 64K/128K/256K
allocations at the moment for hashtables.  When we started looking at
this in bug 1436250, we had a 256K, ~4 128K, and a whole bunch of 64K
hashtable allocs (on linux).  Some may be smaller or gone now, but it's
still big.

I wonder if it's worth the perf hit to realloc to exact size hash tables
that are build-once - probably.  hashtable->Finalize()?  (I wonder if
that would let us make any other memory/speed optimizations if we know
the table is now static.)


I think, as much as possible, we really want static or mostly-static 
hash tables to be shared between processes. I've already been working on this 
in a few areas, e.g., bug 1470365 for string bundles, which are completely 
static, and bug 1471025 for preferences, which are mostly static.


And those patches add helpers which should make it pretty easy to do the same 
for more things in the future, so that should probably be our go-to strategy 
for reducing per-process overhead, when possible.

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


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

2018-07-13 Thread Felipe G
>
>
>
> >Also note that dealing with the "importance" of a page is not just a
> >matter of visibility and focus. There are other factors to take into
> >account such as if the page is playing audio or video (like listening to
> >music on YouTube), if it's self-updating and so on.
>
> Absolutely
>

We should think about how we can make different performance and memory
trade-offs for processes that are hosting top-level frames and processes
hosting 3rd-party subframes

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


Scheduled - [TCW] Scheduled Tree Closing Maintenance Window July 14th

2018-07-13 Thread Kim Moir
As a fyi there will be a tree closing window tomorrow July 14 06:00-15:00PDT

tldr: Trees will be closed for the duration of this maintenance,
bugzilla and mercurial will also be unavailable.




>From https://mozilla2.statuspage.io/incidents/980f0kzk533g

Impact to Users:
Trees will be closed for the duration of this maintenance, bugzilla and
mercurial will also be unavailable. See complete list of work below.

List of work to be completed:

06:00 - ~14:00 PDT:
Bug 1471612 - Bugzilla DB changes (~8 hours)

09:00 - ~13:00 PDT:
Bug 1471319 - hgmo migration from SCL3 to MDC1 (~4 Hours)

~14:00 PST - 15:00 PDT:
Bug 1468437 / CHG0013025 - OpenVPN maintenance (~10 minutes)

Tracker Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1472833

If you have questions about this issue, please email m...@mozilla.com or
visit #moc in IRC. We appreciate your patience while we perform this
maintenance work.

Thank you,
Mozilla Operations Center
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: DNS Rebinding protection

2018-07-13 Thread Tom Ritter
I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1475605 to capture
this issue and (most of) this discussion.

On Tue, Jun 26, 2018 at 5:17 PM, Brannon Dorsey 
wrote:

> >
> > First, I think downright denying "private IP addresses" from DNS
> responses
> > is very hard and is doomed to break the web experience for a set of users
> > who use private/local DNSes etc.
> >
>
> Yes, I concur.  Lets not block them outright, but rather only in the
> instance that a domain name that previously resolved for a public IP is now
> resolving for a private one.
>
> I haven't taken a look at the browser's DNS caching code but I suspect that
> you are correct that we would need to maintain a separate cache
> specifically for the purpose of protecting against this malicious rebinding
> behavior.
>
> Depening on how long we'd want to cache the local-ip-banned host name, it
> > could mean quite a lot of memory... How long would a sensible time be
> > anyway?
> >
>
> We could maintain this cache based on time, but perhaps a better method
> would be to instead tie cache entries with browser windows/tabs and the web
> workers and iframes that they include. Once a domain name has been resolved
> to a public IP from a web page via HTML or JavaScript, or the child pages
> and workers that web page spawns, that domain name should never be
> permitted to resolve to a private IP. These cache entries could then be
> purged when a web page is closed instead of trying to heuristically choose
> an appropriate lifetime in seconds.
>
> PGP Public Key 
> https://brannon.online 
> @brannondorsey 
>
> On Tue, Jun 26, 2018 at 4:25 PM, Daniel Stenberg  wrote:
>
> > On Mon, 25 Jun 2018, Brannon Dorsey wrote:
> >
> > Users can protect themselves from this type of attack by using a DNS
> >> resolver that filters out private IP addresses from public DNS
> responses.
> >> OpenDNS and dd-wrt can both provide this functionality if configured
> >> properly, but my question is, *why not block this type of illegitimate
> and
> >> dangerous DNS behavior at the browser level?*
> >>
> >> I'm interested in discussing the possibility of providing protection
> >> against DNS rebinding in the Firefox browser itself. As far as I see
> it, a
> >> domain name should never be allowed to respond with a private IP address
> >> moments after it first responded with a public IP address.
> >>
> >
> > First, I think downright denying "private IP addresses" from DNS
> responses
> > is very hard and is doomed to break the web experience for a set of users
> > who use private/local DNSes etc.
> >
> > Refusing private addresses for a host name that "recently" returned
> > non-private ones seems like it perhaps could be doable, but would
> introduce
> > an interesting caching challenge since we'd need to keep the "local IP
> ban"
> > around on a per host name basis for a period of time after each host was
> > resolved (with non-local addresses) and the cache would have to be kept
> > idependent of the regular DNS cache. Depening on how long we'd want to
> > cache the local-ip-banned host name, it could mean quite a lot of
> memory...
> > How long would a sensible time be anyway?
> >
> > As a side-note: we already deny RFC1918-addresses from DNS-over-HTTPS
> > responses so in that regard, using TRR will save you from these DNS
> attacks!
> >
> > --
> >
> >  / daniel.haxx.se
> >
> ___
> 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: Fission MemShrink Newsletter #1: What (it is) and Why (it matters to you)

2018-07-13 Thread Randell Jesup
>On 13/07/2018 04:55, Randell Jesup wrote:
>> Correct - we need to have observers/what-have-you for
>> background/foreground state (and we may want an intermediate state or
>> two - foreground-but-not-focused (for example a visible window that
>> isn't the focused window); recently-in-foreground (switching back and
>> forth); background-for-longer-than-delta, etc.
>> 
>> Modules can use these to drop caches, shut down unnecessary threads,
>> change strategies, force GCs/CCs, etc.

>Also note that dealing with the "importance" of a page is not just a
>matter of visibility and focus. There are other factors to take into
>account such as if the page is playing audio or video (like listening to
>music on YouTube), if it's self-updating and so on.

Absolutely

>The only mechanism to reduce memory consumption we have now is
>memory-pressure events which while functional are still under-used. We
>might also need more fine grained mechanisms than "drop as much memory
>as you can".

This is also very important for GeckoView

-- 
Randell Jesup, Mozilla Corp
remove "news" for personal email
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


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

2018-07-13 Thread Randell Jesup
>On Thu, Jul 12, 2018 at 08:56:28AM -0700, Andrew McCreight wrote:
>>On Thu, Jul 12, 2018 at 3:57 AM, Emilio Cobos Álvarez 
>>wrote:
>>
>>> Just curious, is there a bug on file to measure excess capacity on
>>> nsTArrays and hash tables?
[snip]
>I kind of suspect that improving the storage efficiency of hashtables (and
>probably nsTArrays too) will have an out-sized effect on per-process
>memory. Just at startup, for a mostly empty process, we have a huge amount
>of memory devoted to hashtables that would otherwise be shared across a
>bunch of origins—enough that removing just 4 bytes of padding per entry
>would save 87K per process. And that number tends to grow as we populate
>caches that we need for things like layout and atoms.

Hash tables are a big issue.  There are a lot of 64K/128K/256K
allocations at the moment for hashtables.  When we started looking at
this in bug 1436250, we had a 256K, ~4 128K, and a whole bunch of 64K
hashtable allocs (on linux).  Some may be smaller or gone now, but it's
still big.

I wonder if it's worth the perf hit to realloc to exact size hash tables
that are build-once - probably.  hashtable->Finalize()?  (I wonder if
that would let us make any other memory/speed optimizations if we know
the table is now static.)

-- 
Randell Jesup, Mozilla Corp
remove "news" for personal email
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: PSA: pay attention when setting multiple reviewers in Phabricator

2018-07-13 Thread Randell Jesup
>On 05/07/2018 18:19, Mark Côté wrote:
>> I sympathize with the concerns here; however, changing the default would
>> be a very invasive change to Phabricator, which would not only be complex
>> to implement but troublesome to maintain, as we upgrade Phabricator every
>> week or two.

Makes sense.

>> This is, however, something we can address with our new custom
>> commit-series-friendly command-line tool. We are also working towards the
>> superior solution of automatically selecting reviewers based on module
>> owners and peers and enforcing this in Lando.
>
>Automatically selecting reviewers sounds like a huge improvement,
>particularly for people making changes who haven't yet internalised the
>ownership status of the files they are touching (notably any kind of
>first-time or otherwise infrequent contributor to a specific piece of
>code). So I'm very excited about this change.

Agreed that this is an improvement for those who are new to mozilla
development, though the bugzilla reviewer suggestions are also useful
today.  This would be the equivalent of adding "Select for me" in Bugzilla.

>That said, basing it on the list of module owners & peers seems like it may
>not be the right decision for a number of reasons:
>
>* The number of reviews for a given module can be very large and being
>unconditionally selected for every review in a module may be overwhelming.
>
>* The list of module owners and peers is not uniformly well maintained (in
>at least some cases it suggests that components are owned by people who
>have not been involved with the project for several years). Although this
>should certainly be cleaned up, the fact is that the current data is not
>reliable in many cases.

This can be cleaned up (and is being so), but will never be perfect.

Also, unless you're amazingly careful, this will often route reviews to
people on PTO, sick, in the "wrong" time zone, on a holiday day, or
simply too busy to review in a reasonable period due to whatever.  Some
of these are known ahead of time, others aren't.  And then there are the
"I need a review now to fix something" where the automatic selection
might be someone who won't be available to review it for 12 hours, or 3
days - or 3 weeks, requiring noticing this and having someone manually
correct it.

>* Oftentimes there is substructure within a module that means that some
>people should be reviewers in certain files/directories but have no
>knowledge of other parts.

Or you spend a lot of time deciding and updating who can review what
bits - and it doesn't always match the directory heirarchy!

>* It usually desirable to have people perform code review for some time as
>part of the process of becoming a module owner or peer.

Absolutely.

>A better solution would be to have in-tree metadata files providing
>subscription rules for code review (e.g. a mapping of usernames to a list
>of patterns matching files). Module owners would be responsible for
>reviewing changes to these rules to ensure that automatic delegation
>happens to the correct people.

I still don't believe this would work well in practice.  It would work,
but would have frequent problem cases and often require a lot of updating.

In-tree metadata to support suggestions (and to support "select for me"
when someone explicitly asks) is good, I think.  Note that not all
modules are directly related to directories in the tree, so something
out-of-tree or some escape valve in-tree is needed to record those.

I think automatically selecting reviewers always (as is implied, but it
isn't clear that this is the plan - Mark?) is quite problematic, and
enforcing reviewers-are-listed-in-tree also has real problems (and may
require too-frequent manual intervention) such as not supporting peers
assigning reviews to non-peers to train them (to be peers), for cases
where a the best person to review a patch isn't a full module peer but
an SME on the issue/code in question, etc.  These problems can be
gamed/wallpapered (assign a review to both the non-peer and a peer who
doesn't really review or reviews-the-review, etc), but that adds pain
and time and unneeded process.

Some modules (i.e. DOM) already to have a hard requirement for peer
review.  That should be continued and should be enforced as it is now,
and it sounds like Lando will (does?) support that.  If another module
wants to enforce it we can flip a bit in the list of peers and have it
enforced.

-- 
Randell Jesup, Mozilla Corp
remove "news" for personal email
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


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

2018-07-13 Thread David Major
This touches on a really important point: we're not the only ones
allocating memory.

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

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


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


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

2018-07-13 Thread Gabriele Svelto
Just another bit of info to raise awareness on a thorny issue we have to
face if we want to significantly raise the number of content processes.
On 64-bit Windows we often consume significantly more commit space than
physical memory. This consumption is currently unaccounted for in
about:memory though I've seen hints of it being cause by the GPU driver
(or other parts of the graphics pipeline). I've filed bug 1475518 [1] so
that I don't forget and I encourage anybody with Windows experience to
have a look because it's something we _need_ to solve to reduce content
process memory usage.

 Gabriele

[1] Commit-space usage investigation
https://bugzilla.mozilla.org/show_bug.cgi?id=1475518



signature.asc
Description: OpenPGP digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


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

2018-07-13 Thread Gabriele Svelto
On 13/07/2018 04:55, Randell Jesup wrote:
> Correct - we need to have observers/what-have-you for
> background/foreground state (and we may want an intermediate state or
> two - foreground-but-not-focused (for example a visible window that
> isn't the focused window); recently-in-foreground (switching back and
> forth); background-for-longer-than-delta, etc.
> 
> Modules can use these to drop caches, shut down unnecessary threads,
> change strategies, force GCs/CCs, etc.
> 
> Some of this certainly already exists, but may need to be extended (and
> used a lot more).

We already had most of this stuff in the ProcessPriorityManager [1]
which has be only ever used in Firefox OS. Since we had
one-process-per-tab there it was designed that way so it might need some
reworking to deal with one tab consisting of multiple content processes.

Also note that dealing with the "importance" of a page is not just a
matter of visibility and focus. There are other factors to take into
account such as if the page is playing audio or video (like listening to
music on YouTube), if it's self-updating and so on.

The only mechanism to reduce memory consumption we have now is
memory-pressure events which while functional are still under-used. We
might also need more fine grained mechanisms than "drop as much memory
as you can".

 Gabriele

[1]
https://searchfox.org/mozilla-central/rev/46292b1212d2d61d7b5a7df184406774727085b8/dom/ipc/ProcessPriorityManager.cpp



signature.asc
Description: OpenPGP digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform