GNOME Bugzilla – Bug 720298
Resource usage improvements to the calendar
Last modified: 2013-12-16 17:45:12 UTC
This was spotted by Adam Williamson and others in the "Memory Leak" bug. While not necessarily a leak (the GC wasn't being triggered), let's not do any more than we need to, and not rebuild the entire calendar layout every time we open the calendar or set the date.
Created attachment 264034 [details] [review] calendar: Don't ever force reload
Created attachment 264035 [details] [review] calendar: Don't rebuild the entire calendar widget when choosing a date It's inefficient and wasteful. Combined with the JS GC not being great, it leaks around 100 actors waiting to be GC'd. Yikes.
Review of attachment 264034 [details] [review]: So how about event backends that require a pull from the upstream service? Do we expect e-d-s to handle it?
Review of attachment 264035 [details] [review]: ::: js/ui/calendar.js @@ +669,3 @@ + this._monthLabel.text = this._selectedDate.toLocaleFormat(this._headerFormat); + + if (!this._calendarBegin || _sameMonth(this._selectedDate, this._calendarBegin)) Reversed logic? As I read the code, you want to rebuild if the new selected date is in a different month.
Created attachment 264160 [details] [review] calendar: Don't rebuild the entire calendar widget when choosing a date It's inefficient and wasteful. Combined with the JS GC not being great, it leaks around 100 actors waiting to be GC'd. Yikes. Good catch.
Attachment 264034 [details] pushed as 9fce12d - calendar: Don't ever force reload Attachment 264160 [details] pushed as cc4659f - calendar: Don't rebuild the entire calendar widget when choosing a date I asked Matthew Barnes about this, and he said we should be fine.