Tested in Ubuntu 18.04 and this fix works well for me. I no longer see sustained growth in gnome-shell's memory usage when repeatedly entering and leaving the overview.

As we've discussed on IRC, I'm not opposed to this but I would like some more confirmation of how it does or doesn't affect performance, especially on machines that are not programmer-caliber fast.

It is a big hammer, perhaps necessary, but I'd like to keep exploring other options such as !50 (merged) in addition.

FWIW, I've been playing with an even bigger hammer for more than a week now (unconditional GC in gnome-shell leisure handler), and I have barely seen any stalls all this time.

I just did that to reduce noise in heaptrack and never thought of it as a viable option, but it raises my confidence on this patch. The GC would be just as effective with less grinding involved.

This was on an i5-6200U, I guess on the mediocre side of "programmer-grade". Anyhow, I'll try to get this patch up on the baytrail tablet and report back.

@carlosg please let us know how your tablet testing went with this patch.

FTR testing came out good! This is not an specially snappy machine, but framerates stood similar, the usual short stalls (eg. opening Activities) didn't get noticeably longer, and none was observed out of the blue.

I would say "ship it", but also:

"There are merge conflicts. Resolve these conflicts or ask someone with write access to this repository to merge it locally"

Sure, let me update it. Thanks for testing.

Ideally, though, we'd have a more structured way to measure the impact of these changes. None comes to my mind now, but I'm certain that even a quick research would yield meaningful results.

One thought is that we can add "marks" to the Sysprof profile format, and then use them at begin/end of a GC operation. (We'll also need a visualizer in Sysprof for marks).

Then you would just enable the profiler to compare GJS versions.

That would be ideal :). But I don't think the numbers would be really meaningful this time, given the GC strategy we are switching from is basically nonexistent.

Of course would be nice to ensure the median run of a GC doesn't take longer than say half a frame, but I don't think we have such fine grained control. Still the patch seems favorable in that it won't usually let many elements accumulate.

Please rebase again. You're missing critical crash fix 8510bede :)

It shouldn't be necessary to rebase. There's no merge conflict.

@vanvugt can you state here what testing you've done please / on what hardware? I think upstream guys are hoping for more results. :-)

@iainl @vanvugt The most important and interesting information is how much this patch impacts the performance of GJS/GNOME Shell on lower-end hardware. It is somewhat a aggressive approach, since it forces a GC every time an object is destroyed. It might be not as bad as we think because each GC now has a much smaller number of objects to collect in average.

I'd feel most secure with this patchset if this kind of information is provided. I myself will test on a lower hardware of mine, but the more data points we have, the better.

I think the smaller memory footprint is going to benefit low-end hardware more than the slightly increased execution time will hurt (if at all - I can't see any increase right now).

While I'm a big advocate of making everything perform silky-smooth on low-end hardware, I haven't got to doing that with gnome-shell yet. Unfortunately the reason is that we don't have great performance on even high-end hardware yet. So that's my focus at the moment. I'm presently using an i7-7700 desktop, but had previously done most of my gnome-shell work on a Xeon E3-1246 v3.

Ubuntu 18.04 now contains this patch, so we're about to find out how it goes on a large scale: https://launchpad.net/ubuntu/+source/gjs/1.52.1-1ubuntu1

Seems fine. If we do notice stalls during animations, then we could consider rate-limiting the forced GCs to once per 5 frames, as is done with the RSS checks in gjs_gc_if_needed(). But I think the idle function at low priority is probably going to mean that there won't be any forced GCs during animations.

Ubuntu 18.04 now contains this patch, so we're about to find out how it goes on a large scale

That's up to you, but I don't intend to backport this to 1.52 until we run it for a while on 1.53.x. I guess Ubuntu is that testing ground now...