GNOME Bugzilla – Bug 786017
/proc/self/stat is read for every frame if GC was not needed
Last modified: 2017-08-21 23:18:18 UTC
Created attachment 357216 [details] [review] possible patch *not* tested In the special case of dragging a window quite a lot of CPU time seems to be spend in the kernel while reading /proc/self/stat. Looking at a sysprof profile we see something like the below. Of which entry_SYSCALL_64_fastpath appears to be sys_read for reading /proc/self/stat (and e.g. includes some SE linux overhead). SELF CUMULATIVE FUNCTION [ 0.00%] [ 100.00%] [Everything] [ 0.12%] [ 80.24%] [/usr/bin/gnome-shell] [ 0.00%] [ 29.97%] In file [heap] [ 2.60%] [ 21.27%] In file /usr/lib64/libpthread-2.25.so [ 0.00%] [ 18.67%] - - kernel - - [ 0.54%] [ 15.50%] entry_SYSCALL_64_fastpath [ 1.78%] [ 1.78%] entry_SYSCALL_64 [ 1.28%] [ 1.28%] entry_SYSCALL_64_after_swapgs [ 0.10%] [ 0.10%] sys_read [ 0.00%] [ 0.02%] __irqentry_text_start [ 1.68%] [ 1.69%] __pthread_disable_asynccancel [ 1.64%] [ 1.65%] __pthread_enable_asynccancel [ 0.00%] [ 1.34%] ioctl In https://git.gnome.org/browse/gjs/tree/gjs/jsapi-util.cpp#n694 there is throttling for the GC run, one could consider throttling the query for the RSS size.
Review of attachment 357216 [details] [review]: Thanks for the patch. I think it makes sense. This RSS-size-checking code is primarily for the benefit of gnome-shell, so please do test it with gnome-shell and check what the gnome-shell people think too. Especially if you want to get this in for 3.26. ::: gjs/jsapi-util.cpp @@ +775,2 @@ static gulong linux_rss_trigger; +static gint64 last_gc_check_time; As long as you're changing this line, change the type to int64_t since our coding style prefers that.
(In reply to Philip Chimento from comment #1) > This RSS-size-checking code is primarily for the benefit of gnome-shell, > so please do test it with gnome-shell and check what the gnome-shell people > think too. If memory serves me right, the primary case the check addresses is the "old wallpaper issue": An object that uses a significant amount of memory in C is up for garbage collection, but as the memory consumption visible to the JS engine is tiny (just the object wrapper), it doesn't meet the threshold and GC doesn't actually happen. I don't see how throttling the check would break that case, so the change is fine from my POV. (And fwiw, I've been running with the patch applied for a couple of days now)
OK, that's pretty much what I thought, I'll commit this then. (Benjamin, you get a freebie this time, I'll fix up the comments so I can include this in 1.49.91 today :-)