Re: Memory management in features implemented in JS
Ehsan Akhgari writes: > On 2014-03-19, 10:50 PM, Boris Zbarsky wrote: >> On 3/19/14 10:40 PM, Ehsan Akhgari wrote: >>> Why do we have to touch that list on shutdown? >> >> We Release() all the things in it (nsIWeakReferences in this case). > > Well, that was sort of my point (gotta work on my style of being > overly terse, sorry about that!). Why do we need to do that? > > I just checked and the implementations of the only two > implementations of that interface (nsWeakReference and > nsNodeWeakReference) do not have any side effects which will > result in a write to disk. So I don't see if we just avoid > clearing the observer list hashtable at shutdown (I mean, besides > keeping it enabled in debug builds of course.) Sounds like there is an optimization that can be applied there, but that is only half the problem. The leaked small objects fragment the available heap. The observer service, and other users of weak references, need notifications when the objects are destroyed. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Memory management in features implemented in JS
On 2014-03-19, 10:50 PM, Boris Zbarsky wrote: On 3/19/14 10:40 PM, Ehsan Akhgari wrote: Why do we have to touch that list on shutdown? We Release() all the things in it (nsIWeakReferences in this case). Well, that was sort of my point (gotta work on my style of being overly terse, sorry about that!). Why do we need to do that? I just checked and the implementations of the only two implementations of that interface (nsWeakReference and nsNodeWeakReference) do not have any side effects which will result in a write to disk. So I don't see if we just avoid clearing the observer list hashtable at shutdown (I mean, besides keeping it enabled in debug builds of course.) That said, the "minutes" cases are the ones where the notification is one that's actually fired at shutdown, because then we start removing the observers from the list, etc... Ah... That actually rings a bell, I think. :-) Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Memory management in features implemented in JS
On 3/19/14 10:40 PM, Ehsan Akhgari wrote: Why do we have to touch that list on shutdown? We Release() all the things in it (nsIWeakReferences in this case). That said, the "minutes" cases are the ones where the notification is one that's actually fired at shutdown, because then we start removing the observers from the list, etc... -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Memory management in features implemented in JS
On 2014-03-19, 10:25 PM, Boris Zbarsky wrote: On 3/19/14 9:41 PM, Justin Dolske wrote: It uses a weak reference with the observer service, plus a dummy strong reference (via addEventListener()) to automatically manage the lifetime... When the node/document does away, so does the event listener. This is sort of ok for notifications that fire a lot, but for notifications that fire rarely, or never, this effectively leaks a word of memory in the observer service. And then they all get touched at shudown. We've had bugs with hundreds of thousands of dead weak observers all hanging out in the observer service making shutdown take minutes. :( Why do we have to touch that list on shutdown? ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Memory management in features implemented in JS
On 3/19/14 9:41 PM, Justin Dolske wrote: It uses a weak reference with the observer service, plus a dummy strong reference (via addEventListener()) to automatically manage the lifetime... When the node/document does away, so does the event listener. This is sort of ok for notifications that fire a lot, but for notifications that fire rarely, or never, this effectively leaks a word of memory in the observer service. And then they all get touched at shudown. We've had bugs with hundreds of thousands of dead weak observers all hanging out in the observer service making shutdown take minutes. :( For cases like the one linked to there, it really would be good to have a better option than dealing with the observer service... -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Memory management in features implemented in JS
On 2014-03-19, 7:58 PM, Kyle Huey wrote: On Wed, Mar 19, 2014 at 4:52 PM, David Rajchenbach-Teller wrote: Couldn't we insist that any reference to a DOM element in JS (or at least in JS-implemented components) must be a weak pointer-style wrapper? I seem to remember that we have introduced something along these lines a few years ago as a way to fight against leaks of references to DOM by add-ons. On 3/20/14 12:39 AM, Kyle Huey wrote: Followup to dev-platform please. -- David Rajchenbach-Teller, PhD Performance Team, Mozilla The issue is not leaking the DOM elements, it's leaking the (chrome) JS implementations themselves. Wrapper cutting actually masks these leaks by stopping them from entraining entire windows past the tests of these features. Does it make sense to disable wrapper cutting in (let's say) debug builds, so that at least we catch some of these badness? Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: DXR gets multi-line highlighting
On 19/03/2014 10:20, smaug wrote: > mxr can do specific revision + multiline. > Something like > http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.h?rev=492ce94e6528&mark=94,96,98,100-113,120-134#90 > > You get the revision number from the bottom of the page. (That UI isn't too > great.) Aaargh! And where is this documented? If I had known about this Phil -- Philip Chee , http://flashblock.mozdev.org/ http://xsidebar.mozdev.org Guard us from the she-wolf and the wolf, and guard us from the thief, oh Night, and so be good for us to pass. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Memory management in features implemented in JS
On 3/19/14 4:39 PM, Kyle Huey wrote: We are discovering a lot of leaks in JS implemented DOM objects. The general pattern seems to be that we have a DOM object that also needs to listen to events from the message manager or notifications from the observer service, which usually hold strong references to the listener. This little hack works for the observer service: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-plugins.js#1226 It uses a weak reference with the observer service, plus a dummy strong reference (via addEventListener()) to automatically manage the lifetime... When the node/document does away, so does the event listener. Dunno if the message manager has a way to do this, but perhaps it wouldn't be hard to add. That it, it's kind of a hack and a cleaner solution would be nice. Justin ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Intent to ship: CSS Variables
smaug wrote:
Curious, how much have we tested the performance of our implementation
and are there some known perf issues?
I have not tested. The only extra work we should be doing at the moment
when variables aren't used in the style sheet is
nsRuleNode::ResolveVariableReferences. That iterates over all of the
properties we are computing for a given frame and checks to see if they
are a token stream, and if so, reparses them.
If this is a problem we could easily record on the
nsCSS{Compressed,Expanded}DataBlock object during the initial parsing
whether a property with a variable reference on the right hand side was
encountered, and copy that over when we map the properties into the
nsRuleData. Then we could skip the ResolveVariableReferences call if we
knew we didn't find any properties with variable references.
When variables are used, we do end up having to parse a
variable-reference-using property value twice (once to ensure that
variable references are syntactically valid and to record which
variables are referenced, and another time after expanding out the
variable references), but that is basically what is required by the spec.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Re: Memory management in features implemented in JS
On 03/20/2014 02:25 AM, smaug wrote: On 03/20/2014 01:58 AM, smaug wrote: On 03/20/2014 01:39 AM, Kyle Huey wrote: Followup to dev-platform please. We are discovering a lot of leaks in JS implemented DOM objects. The general pattern seems to be that we have a DOM object that also needs to listen to events from the message manager or notifications from the observer service, which usually hold strong references to the listener. In C++ we would split the object into two separate refcounted objects, one for the DOM side and another that interfaces with these global singletons (which I'll call the proxy). Between this pair of objects at least one direction would be weak, allowing the DOM object's lifetime to be managed appropriately by the garbage and cycle collectors and in its destructor it could tell the proxy to unregister itself and die. But this isn't possible because of the lack of weak references in JS. Instead we end up running a bunch of manual cleanup code at inner-window-destroyed. This is already bad for many things because it means that our object now lives for the lifetime of the window (which might be forever, in the case of the system app window on B2G). Also in some cases we forget to remove our inner-window-destroyed observer so we live forever. For objects not intended to live for the lifetime of the window we need to manually perform the same cleanup when we figure out that we can go away (which can be quite difficult since we can't usefully answer the question "is something in the content page holding me alive?"). All of this requires a lot of careful manual memory management which is very easy to get wrong and is foreign to many JS authors. Short of not implementing things in JS, what ideas do people have for fixing these issues? We have some ideas of how to add helpers to scope these things to the lifetime of the window (perhaps by adding an API that returns a promise that is resolved at inner-window-destroyed to provide a good cleanup hook that is not global) but that doesn't help with objects intended to have shorter lifetimes. Is it possible for us to implement some sort of useful weak reference in JS? I'm rather strongly against adding weak refs to the web platform. They expose GC behavior, which lead to odd and hard to debug errors, and implementations may have to use pretty much the same GC. Internally we have weakref support in WrappedJS, and one can use weak observers for ObserverService and weak listeners for MessageManager. Aren't those not enough for the cases where weakrefs anyway can help? We could use WrappedJS also in some kind of chrome Promises - WeakPromise, which wouldn't keep the callback alive. -Olli And we could add a flag to WrappedJS so that it would call some callback when it is about to go away. That would let cleanup of WeakPromise happen asap. Basically keep a hashtable wrappedjs -> objects_implementing_callback_interface_foo. Or that should be wrappedjs -> array Then when adding an object to the hashtable, wrappedjs would get marked with a flag, that it should look at the hashtable when it is going away, and if there is value in the hashtable, call the foo. This cleanup mechanism could be used with weak observers and weak listeners too. -Olli ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Memory management in features implemented in JS
On 03/20/2014 01:58 AM, smaug wrote: On 03/20/2014 01:39 AM, Kyle Huey wrote: Followup to dev-platform please. We are discovering a lot of leaks in JS implemented DOM objects. The general pattern seems to be that we have a DOM object that also needs to listen to events from the message manager or notifications from the observer service, which usually hold strong references to the listener. In C++ we would split the object into two separate refcounted objects, one for the DOM side and another that interfaces with these global singletons (which I'll call the proxy). Between this pair of objects at least one direction would be weak, allowing the DOM object's lifetime to be managed appropriately by the garbage and cycle collectors and in its destructor it could tell the proxy to unregister itself and die. But this isn't possible because of the lack of weak references in JS. Instead we end up running a bunch of manual cleanup code at inner-window-destroyed. This is already bad for many things because it means that our object now lives for the lifetime of the window (which might be forever, in the case of the system app window on B2G). Also in some cases we forget to remove our inner-window-destroyed observer so we live forever. For objects not intended to live for the lifetime of the window we need to manually perform the same cleanup when we figure out that we can go away (which can be quite difficult since we can't usefully answer the question "is something in the content page holding me alive?"). All of this requires a lot of careful manual memory management which is very easy to get wrong and is foreign to many JS authors. Short of not implementing things in JS, what ideas do people have for fixing these issues? We have some ideas of how to add helpers to scope these things to the lifetime of the window (perhaps by adding an API that returns a promise that is resolved at inner-window-destroyed to provide a good cleanup hook that is not global) but that doesn't help with objects intended to have shorter lifetimes. Is it possible for us to implement some sort of useful weak reference in JS? I'm rather strongly against adding weak refs to the web platform. They expose GC behavior, which lead to odd and hard to debug errors, and implementations may have to use pretty much the same GC. Internally we have weakref support in WrappedJS, and one can use weak observers for ObserverService and weak listeners for MessageManager. Aren't those not enough for the cases where weakrefs anyway can help? We could use WrappedJS also in some kind of chrome Promises - WeakPromise, which wouldn't keep the callback alive. -Olli And we could add a flag to WrappedJS so that it would call some callback when it is about to go away. That would let cleanup of WeakPromise happen asap. Basically keep a hashtable wrappedjs -> objects_implementing_callback_interface_foo. Then when adding an object to the hashtable, wrappedjs would get marked with a flag, that it should look at the hashtable when it is going away, and if there is value in the hashtable, call the foo. This cleanup mechanism could be used with weak observers and weak listeners too. -Olli ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Memory management in features implemented in JS
On 03/20/2014 01:39 AM, Kyle Huey wrote: Followup to dev-platform please. We are discovering a lot of leaks in JS implemented DOM objects. The general pattern seems to be that we have a DOM object that also needs to listen to events from the message manager or notifications from the observer service, which usually hold strong references to the listener. In C++ we would split the object into two separate refcounted objects, one for the DOM side and another that interfaces with these global singletons (which I'll call the proxy). Between this pair of objects at least one direction would be weak, allowing the DOM object's lifetime to be managed appropriately by the garbage and cycle collectors and in its destructor it could tell the proxy to unregister itself and die. But this isn't possible because of the lack of weak references in JS. Instead we end up running a bunch of manual cleanup code at inner-window-destroyed. This is already bad for many things because it means that our object now lives for the lifetime of the window (which might be forever, in the case of the system app window on B2G). Also in some cases we forget to remove our inner-window-destroyed observer so we live forever. For objects not intended to live for the lifetime of the window we need to manually perform the same cleanup when we figure out that we can go away (which can be quite difficult since we can't usefully answer the question "is something in the content page holding me alive?"). All of this requires a lot of careful manual memory management which is very easy to get wrong and is foreign to many JS authors. Short of not implementing things in JS, what ideas do people have for fixing these issues? We have some ideas of how to add helpers to scope these things to the lifetime of the window (perhaps by adding an API that returns a promise that is resolved at inner-window-destroyed to provide a good cleanup hook that is not global) but that doesn't help with objects intended to have shorter lifetimes. Is it possible for us to implement some sort of useful weak reference in JS? I'm rather strongly against adding weak refs to the web platform. They expose GC behavior, which lead to odd and hard to debug errors, and implementations may have to use pretty much the same GC. Internally we have weakref support in WrappedJS, and one can use weak observers for ObserverService and weak listeners for MessageManager. Aren't those not enough for the cases where weakrefs anyway can help? We could use WrappedJS also in some kind of chrome Promises - WeakPromise, which wouldn't keep the callback alive. -Olli ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Memory management in features implemented in JS
On Wed, Mar 19, 2014 at 4:52 PM, David Rajchenbach-Teller wrote: > Couldn't we insist that any reference to a DOM element in JS (or at > least in JS-implemented components) must be a weak pointer-style > wrapper? I seem to remember that we have introduced something along > these lines a few years ago as a way to fight against leaks of > references to DOM by add-ons. > > On 3/20/14 12:39 AM, Kyle Huey wrote: >> Followup to dev-platform please. > > > -- > David Rajchenbach-Teller, PhD > Performance Team, Mozilla The issue is not leaking the DOM elements, it's leaking the (chrome) JS implementations themselves. Wrapper cutting actually masks these leaks by stopping them from entraining entire windows past the tests of these features. - Kyle ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Memory management in features implemented in JS
Couldn't we insist that any reference to a DOM element in JS (or at least in JS-implemented components) must be a weak pointer-style wrapper? I seem to remember that we have introduced something along these lines a few years ago as a way to fight against leaks of references to DOM by add-ons. On 3/20/14 12:39 AM, Kyle Huey wrote: > Followup to dev-platform please. -- David Rajchenbach-Teller, PhD Performance Team, Mozilla ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Memory management in features implemented in JS
Followup to dev-platform please. We are discovering a lot of leaks in JS implemented DOM objects. The general pattern seems to be that we have a DOM object that also needs to listen to events from the message manager or notifications from the observer service, which usually hold strong references to the listener. In C++ we would split the object into two separate refcounted objects, one for the DOM side and another that interfaces with these global singletons (which I'll call the proxy). Between this pair of objects at least one direction would be weak, allowing the DOM object's lifetime to be managed appropriately by the garbage and cycle collectors and in its destructor it could tell the proxy to unregister itself and die. But this isn't possible because of the lack of weak references in JS. Instead we end up running a bunch of manual cleanup code at inner-window-destroyed. This is already bad for many things because it means that our object now lives for the lifetime of the window (which might be forever, in the case of the system app window on B2G). Also in some cases we forget to remove our inner-window-destroyed observer so we live forever. For objects not intended to live for the lifetime of the window we need to manually perform the same cleanup when we figure out that we can go away (which can be quite difficult since we can't usefully answer the question "is something in the content page holding me alive?"). All of this requires a lot of careful manual memory management which is very easy to get wrong and is foreign to many JS authors. Short of not implementing things in JS, what ideas do people have for fixing these issues? We have some ideas of how to add helpers to scope these things to the lifetime of the window (perhaps by adding an API that returns a promise that is resolved at inner-window-destroyed to provide a good cleanup hook that is not global) but that doesn't help with objects intended to have shorter lifetimes. Is it possible for us to implement some sort of useful weak reference in JS? - Kyle ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: DXR gets multi-line highlighting
On Thu, Mar 20, 2014 at 10:26:31AM +1300, Karl Tomlinson wrote: > Erik Rose writes: > > > Once we have request-time rendering of indexed content, it will > > be a lot more straightforward to fetch and render arbitrary revs > > out of version control the same way. We just won't have > > analysis—only syntax coloring—since we won't have time to build > > the whole project during the request. ;-) MXR's ability to > > half-ass an analysis at request time is a deeper question with > > no clear answer, short of implementing an entire second analysis > > engine that operates more heuristically. DXR's strength right > > now is its exactness. Ideas are most welcome. > > Yes, I think there are two different purposes here. > > 1) For development, usually I just want the latest code, to chase >references/overrides/definitions/declarations/documentation >etc. around the code. > > 2) For permalinks to code from bugzilla, usually the main aim is >to identify what the code that is being discussed in a comment. > > There are occasions, such as constructing branch patches or > understanding how things worked in the past, where it would be > neat to have all the power of DXR in past snapshots, but this is > less common. I understand that the cost of performing and/or > archiving an analysis for every revision means this is not > feasible. Note that for this, analysis of the tip of each of -release, -beta, -aurora and -esr* would be enough. That's significantly more than just doing -central, but it's also significantly less than doing every single revision. Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: DXR gets multi-line highlighting
Erik Rose writes: > Once we have request-time rendering of indexed content, it will > be a lot more straightforward to fetch and render arbitrary revs > out of version control the same way. We just won't have > analysis—only syntax coloring—since we won't have time to build > the whole project during the request. ;-) MXR's ability to > half-ass an analysis at request time is a deeper question with > no clear answer, short of implementing an entire second analysis > engine that operates more heuristically. DXR's strength right > now is its exactness. Ideas are most welcome. Yes, I think there are two different purposes here. 1) For development, usually I just want the latest code, to chase references/overrides/definitions/declarations/documentation etc. around the code. 2) For permalinks to code from bugzilla, usually the main aim is to identify what the code that is being discussed in a comment. There are occasions, such as constructing branch patches or understanding how things worked in the past, where it would be neat to have all the power of DXR in past snapshots, but this is less common. I understand that the cost of performing and/or archiving an analysis for every revision means this is not feasible. For 2, I don't mind which server permalinks point to, but it would be handy to be able to generate a permalink from DXR. Perhaps this could even be a link to MXR, if that doesn't seem too odd. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Intent to ship: CSS Variables
On 03/18/2014 11:26 AM, Cameron McCormack wrote: CSS Variables is a feature that allows authors to define custom properties that cascade and inherit in the same way that regular properties do, and to reference the values of these custom properties in the values of regular properties (and other custom properties too). http://dev.w3.org/csswg/css-variables/ I blogged about the feature when it initially landed here: http://mcc.id.au/blog/2013/12/variables It lives behind the layout.css.variables.enabled pref, which is currently enabled by default only on Aurora/Nightly. One thing from the specification that we don't implement is the CSSVariableMap, but Tab tells me that this is going to be removed from the spec. I intend to enable this feature by default this Friday (March 21). That would target Firefox 31. The spec has been stable for a while, and the CSS Working Group already has a resolution to publish the document as a Candidate Recommendation. There has been a last minute proposal for a change to the syntax of custom property declarations, to align with other custom features in CSS that are coming down the pipeline (such as custom media queries). There are a couple of proposals on the table: to replace the "var-" prefix of the name of the custom property with a "_" prefix, or perhaps just anywhere within the name. Similarly for "--" rather than "_". Tab wants to discuss this in this week's CSS Working Group telcon, so if the decision is made quickly enough we can update our implementation. The CSS Working Group is aware of our plans to ship. Bug to enable this feature: https://bugzilla.mozilla.org/show_bug.cgi?id=957833 Blink until recently had an implementation of CSS Variables, behind a flag, but not one that supported the fallback syntax (or CSSVariableMap). They have recently removed it saying that they need to rewrite as it had poor performance, but they say they are still interested in the feature Curious, how much have we tested the performance of our implementation and are there some known perf issues? -Olli ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: DXR gets multi-line highlighting
Whee, I get to be a wet blanket! We don't even have any request-writable storage at the moment. SQLite is horrible with write concurrency (and is on an NFS share, to make matters worse), and straight NFS-based writing (sans SQL) would have to be guarded with locks. Even if we solved those problems, complete rendered pages are currently stored. That means any saved "permalinked" page would be stuck with its original set of menus and other markup despite future changes to the UI, leading it to become more inconsistent (and quite possibly nonfunctional as JS's assumptions about DOM structure are broken) over time. Thus, I think the real solution is to hold off until DXR stops plunking 25GB of rendered HTML on NFS and starts rendering it dynamically. That's slated for sometime after next quarter's transition to an Elasticsearch backend, as it depends technically on that. Once we have request-time rendering of indexed content, it will be a lot more straightforward to fetch and render arbitrary revs out of version control the same way. We just won't have analysis—only syntax coloring—since we won't have time to build the whole project during the request. ;-) MXR's ability to half-ass an analysis at request time is a deeper question with no clear answer, short of implementing an entire second analysis engine that operates more heuristically. DXR's strength right now is its exactness. Ideas are most welcome. Erik On Mar 18, 2014, at 22:20 , smaug wrote: > mxr can do specific revision + multiline. > Something like > http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.h?rev=492ce94e6528&mark=94,96,98,100-113,120-134#90 > > You get the revision number from the bottom of the page. (That UI isn't too > great.) ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
