Skip to content

Commit f5801ca

Browse files
committed
Fix performance issues and various bugs
1 parent a999e1c commit f5801ca

9 files changed

Lines changed: 45 additions & 35 deletions

File tree

AUDIT_REPORT.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ Status: ⬜ Needs Fix · ✅ Fixed · ❌ Won't Fix
269269
| **P7** | [sessionManager.js](worksets@blipk.xyz/sessionManager.js#L605-L649) | 605-649 | **Full app rescan on every `getFavorites()` call**. `scanInstalledApps()` iterates every installed app (hundreds including Flatpak/Snap) to rebuild `allApps` map. Called from favorites change signal, `newWorkset`, `newSession`, `_validateSession`. | Cache `scanInstalledApps()` result on first call; invalidate and rescan only when `Shell.AppSystem` emits `"installed-changed"` signal. ||
270270
| **P8** | [panelIndicator.js](worksets@blipk.xyz/panelIndicator.js#L671-L691) | 671-691 | **Full menu teardown + rebuild + sync disk I/O on every open**. `_refreshMenu()` destroys all menu items, recreates all from scratch, then calls `saveSession()` (sync disk write). | Remove `saveSession()` from `_refreshMenu()`. Update existing menu items in-place (change labels, toggle states) instead of full teardown/rebuild; only add/remove entries when the workset list actually changes. | ✅ saveSession removed from _refreshMenu |
271271
| **P9** | [workspaceView.js](worksets@blipk.xyz/workspaceView.js#L249-L251) | 249-251, 294-296 | **Meta.Background GPU memory leak**. Old `Meta.Background` objects are `delete`d (JS property removal) but never `destroy()`d. GPU textures remain allocated until GC collects the GObject (which may never happen due to reference cycles). | Call `.destroy()` on old `Meta.Background` objects before replacing them. Replace `delete` (JS property removal) with explicit `.destroy()` followed by nulling the reference. ||
272-
| **P10** | [uiUtils.js](worksets@blipk.xyz/lib/ui/uiUtils.js#L210-L254) | 210-254 | **Unbounded image cache**. Module-level `knownImages` object caches every wallpaper image (CPU pixbuf + GPU St.ImageContent) forever -- never invalidated, never evicted, never cleared on disable/enable. | Clear the `knownImages` cache on extension disable. Add LRU eviction with a maximum entry count (e.g., 20 images) to prevent unbounded memory growth during long sessions. | |
272+
| **P10** | [uiUtils.js](worksets@blipk.xyz/lib/ui/uiUtils.js#L210-L254) | 210-254 | **Unbounded image cache**. Module-level `knownImages` object caches every wallpaper image (CPU pixbuf + GPU St.ImageContent) forever -- never invalidated, never evicted, never cleared on disable/enable. | Clear the `knownImages` cache on extension disable. Add LRU eviction with a maximum entry count (e.g., 20 images) to prevent unbounded memory growth during long sessions. | |
273273
| **P11** | [workspaceView.js](worksets@blipk.xyz/workspaceView.js#L237-L271) | 237-271 | **Meta.Background created on every workspace gesture**. `refreshDesktop()` triggered from `WorkspaceGroup._init` injection recreates all background managers for ALL workspace groups on every switch animation. | Cache `BackgroundManager` instances per monitor/workspace (same as P1). Add dirty flags and only recreate when the wallpaper path has actually changed; skip recreation if the existing background matches the target. ||
274274

275275
### MEDIUM
@@ -280,14 +280,14 @@ Status: ⬜ Needs Fix · ✅ Fixed · ❌ Won't Fix
280280
| **P13** | [workspaceManager.js](worksets@blipk.xyz/workspaceManager.js#L126-L146) | 126-146 | **No debouncing on workspace switch cascade**. Rapid switching (keyboard, gestures) triggers the full `_activeWorkspaceChanged()` -> `displayWorkset()` -> `setFavorites()` + `setBackground()` + `saveSession()` + `refreshOverview()` chain repeatedly. | Add `GLib.timeout_add()` debounce (100-200ms) to `_activeWorkspaceChanged()`, storing the timeout ID and canceling any previous pending timeout with `GLib.Source.remove()` on each new signal. | |
281281
| **P14** | [sessionManager.js](worksets@blipk.xyz/sessionManager.js#L390-L395) | 390-395 | **O(n^2) duplicate detection** in `_validateSession()`. Inside a `forEach` over worksets, `JSON.stringify` is called on every other workset for comparison. Also a correctness bug: only the last iteration's filter result is kept. | Use a `Set` of `JSON.stringify()`-ed workset values for O(1) duplicate lookup. Accumulate seen hashes in the Set and filter out duplicates in a single pass. Fix the correctness bug by assigning the filter result back to the array. ||
282282
| **P15** | [extension.js](worksets@blipk.xyz/extension.js#L74-L89) | 74-89 | **New `Gio.Settings` created per call**. `dash2panelSettings()` and `dash2dockSettings()` are factory functions, not cached. Each call creates a new dconf-backed Settings object. Signal handlers connected to these ephemeral objects may be ineffective or leak. | Cache the `Gio.Settings` objects as instance properties (e.g., `this._dash2panelSettings`) on first call and return the cached instance on subsequent calls. Null out on `disable()`. ||
283-
| **P16** | [sessionManager.js](worksets@blipk.xyz/sessionManager.js#L203-L229) | 203-229 | **`saveSession()` called inside `forEach` loop** on background/options GSettings change signals. The `return` inside `forEach` only skips the current iteration, not the loop. | Move `saveSession()` outside the `forEach` loop to execute once after all worksets have been updated. The MD5 hash comparison will prevent redundant disk writes anyway. | |
283+
| **P16** | [sessionManager.js](worksets@blipk.xyz/sessionManager.js#L203-L229) | 203-229 | **`saveSession()` called inside `forEach` loop** on background/options GSettings change signals. The `return` inside `forEach` only skips the current iteration, not the loop. | Move `saveSession()` outside the `forEach` loop to execute once after all worksets have been updated. The MD5 hash comparison will prevent redundant disk writes anyway. | |
284284
| **P17** | [workspaceView.js](worksets@blipk.xyz/workspaceView.js#L165-L188) | 165-188 | **String operations on every animation frame**. Overview state handler runs string split + parseFloat + parseInt on `notify::value` which fires at 60fps during gestures. Also mutates GNOME Shell's internal `adjustment.lastValue`. | Use `Math.floor(value)` and `value % 1` for decimal extraction instead of string split/parse. Cache the last integer state and skip processing if unchanged. Stop mutating `adjustment.lastValue`. ||
285285
| **P18** | [workspaceView.js](worksets@blipk.xyz/workspaceView.js#L357-L363) | 357-363 | **Overlay box fully rebuilt every refresh**. `updateOverlay()` destroys all children and the box itself, then recreates all labels, buttons, and layouts from scratch on every overview state change. | Check if the workset name changed before destroying; reuse the existing overlay box and update only label text with `set_text()` when content hasn't changed. Only rebuild when the workset mapping actually changes. | ❌ caching caused stale overlays; rebuild is cheap compared to GPU work |
286286
| **P19** | [workspaceView.js](worksets@blipk.xyz/workspaceView.js#L59) | 59, 433 | **Menus array grows without cleanup**. Popup menus pushed to `this.menus` are never removed or destroyed. `destroy()` method doesn't clean them up. Leaks Clutter actors added to `Main.uiGroup`. | Destroy all menus in `this.menus` during `WorkspaceViewManager.destroy()`. Connect to each menu's `"destroy"` signal to remove it from the array, or use `WeakRef` wrappers to auto-expire dead references. ||
287287
| **P20** | [workspaceIsolater.js](worksets@blipk.xyz/workspaceIsolater.js#L128-L143) | 128-143 | **Full dash redisplay + all-app state notify on every restack**. `refresh()` calls `app.notify("state")` for every running app and `dash._queueRedisplay()` on `"restacked"` which fires on every window create/destroy/move/z-change. | Add `GLib.timeout_add()` debounce (50ms) to `refresh()`, storing the timeout ID and canceling previous pending calls with `GLib.Source.remove()` before scheduling a new one. ||
288288
| **P21** | [panelIndicator.js](worksets@blipk.xyz/panelIndicator.js#L317-L337) | 317-337 | **Double add+move for menu items**. Items are added at position then moved redundantly, causing doubled Clutter layout invalidation and reflow. | Use `addMenuItem(item, position)` with the explicit position argument directly instead of adding then moving. Remove the redundant `moveMenuItem()` calls. ||
289289
| **P22** | [uiUtils.js](worksets@blipk.xyz/lib/ui/uiUtils.js#L168-L186) | 168-186 | **Unbounded tooltip timer accumulation**. Every hover creates 2 `GLib.timeout_add` entries stored in `Me.session.signals`. Old entries persist across menu rebuilds, growing without bound during long sessions. | Store timeout IDs on the widget object itself and remove old timers with `GLib.Source.remove()` before creating new ones. Clean up all tooltip timers when the parent menu item is destroyed. ||
290-
| **P23** | [uiUtils.js](worksets@blipk.xyz/lib/ui/uiUtils.js#L221-L228) | 221-228 | **Synchronous pixbuf decode on compositor thread**. `GdkPixbuf.Pixbuf.new_from_file()` blocks the main thread while decoding images. For 4K+ wallpapers, visible frame drops. | Use `GdkPixbuf.Pixbuf.new_from_stream_async()` with a `Gio.FileInputStream`, or defer loading to idle with `Meta.later_add(Meta.LaterType.IDLE, callback)` so image decoding doesn't block frame rendering. | |
290+
| **P23** | [uiUtils.js](worksets@blipk.xyz/lib/ui/uiUtils.js#L221-L228) | 221-228 | **Synchronous pixbuf decode on compositor thread**. `GdkPixbuf.Pixbuf.new_from_file()` blocks the main thread while decoding images. For 4K+ wallpapers, visible frame drops. | Use `GdkPixbuf.Pixbuf.new_from_stream_async()` with a `Gio.FileInputStream`, or defer loading to idle with `Meta.later_add(Meta.LaterType.IDLE, callback)` so image decoding doesn't block frame rendering. | ✅ uses new_from_file_at_scale to decode at display size (150px) instead of full resolution |
291291
| **P24** | [sessionManager.js](worksets@blipk.xyz/sessionManager.js#L467) | 467 | **Redundant `refreshOverview()` after save**. Called inside `saveSession()`, but many callers also call it afterward, resulting in double refreshes. | Remove `refreshOverview()` from inside `saveSession()`. Let callers explicitly call `refreshOverview()` only when they need a UI update, avoiding redundant double refreshes. ||
292292
| **P25** | [utils.js](worksets@blipk.xyz/utils.js#L269-L281) | 269-281 | **Sparse array for signal IDs**. GLib source IDs can be large integers, creating arrays with thousands of empty slots. Should use `Map`. | Replace the sparse `this.signalIds[]` array with `new Map()` using `.set(id, target)` / `.get(id)` / `.delete(id)` for O(1) lookup without memory gaps. ||
293293

@@ -301,11 +301,11 @@ Status: ⬜ Needs Fix · ✅ Fixed · ❌ Won't Fix
301301
| P29 | panelIndicator.js | 702-708 | `_worksetMenuItemMoveToTop` adds item then `_refreshMenu()` destroys and rebuilds all | Use `moveMenuItem()` directly on the existing menu item to reposition it, instead of re-creating it and triggering a full `_refreshMenu()` rebuild. | |
302302
| P30 | dialogs.js | throughout | `Array(x).map()` anti-pattern used as closure scope (6 instances) | Replace `Array(x).map(...)` with direct code blocks or IIFEs `(() => { ... })()` where scoping is needed. The Array constructor creates sparse arrays that `map()` skips over. | ❌ working as intended |
303303
| P31 | dialogs.js | 298-300 | Unused deep copy of `editableObject` (`_unreferencedObjectCopy` never read) | Remove the unused `_unreferencedObjectCopy = JSON.parse(JSON.stringify(editableObject))` line entirely, or implement cancel/revert functionality using it if that was the original intent. ||
304-
| P32 | uiUtils.js | 89-95 | `destroyIconButtons` has `destroy()` call commented out -- relies on parent cleanup | Uncomment the `iconButton.destroy()` call in `destroyIconButtons()` to properly clean up Clutter actors and prevent memory leaks when menus are recreated. | |
304+
| P32 | uiUtils.js | 89-95 | `destroyIconButtons` has `destroy()` call commented out -- relies on parent cleanup | Uncomment the `iconButton.destroy()` call in `destroyIconButtons()` to properly clean up Clutter actors and prevent memory leaks when menus are recreated. | ✅ already fixed in codebase |
305305
| P33 | uiUtils.js | 68 | `iconsButtonsPressIds` initialized to `iconButtons` (shared reference bug) | Initialize `parentItem.iconsButtonsPressIds = []` as a new empty array instead of assigning the `iconButtons` array reference, which causes the two arrays to share the same backing store. | ✅ already fixed in codebase |
306306
| P34 | dev.js | 76-82 | O(n^2) cycle detection in `JSON.stringify` replacer using `indexOf` instead of `Set` | Replace `seen.indexOf(val)` with a `new Set()` using `seen.has(val)` / `seen.add(val)` for O(1) cycle detection in the JSON.stringify replacer. ||
307307
| P35 | workspaceView.js | 303-311 | `add_child` called on label that may already be a child (causes Clutter warning) | Add a guard: check `if (thumbnailBox.worksetLabel?.get_parent()) return` before calling `add_child()`, or use `replace_child()` when updating an existing label. ||
308-
| P36 | shader.js | 14-28 | GLSL source string re-allocated on each `vfunc_get_static_shader_source()` call | Extract the GLSL source string to a module-level `const SHADER_SOURCE = "..."` constant and return it from the function to avoid re-allocating on every paint call. | |
308+
| P36 | shader.js | 14-28 | GLSL source string re-allocated on each `vfunc_get_static_shader_source()` call | Extract the GLSL source string to a module-level `const SHADER_SOURCE = "..."` constant and return it from the function to avoid re-allocating on every paint call. | |
309309

310310
---
311311

_package.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ rm -f ./worksets@blipk.xyz/schemas/gschemas.compiled
55
glib-compile-schemas ./worksets@blipk.xyz/schemas
66
rm -rf worksets@blipk.xyz.zip
77
cd worksets@blipk.xyz
8-
bsdtar -a -cf ../worksets@blipk.xyz.zip *
8+
bsdtar -a -cf ../worksets@blipk.xyz.zip --exclude 'lib/ui/shader.js' *
99
cd ..
1010
#zip worksets@blipk.xyz.zip install.sh 'Install Customised Workspaces.desktop'

prompt.txt

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,3 @@ You can find the gnome shell associated libraries documentations here: https://g
66

77
You can find the gnome-shell source code here if you need to analyze it: https://github.com/GNOME/gnome-shell/tree/main/js/ui
88

9-
10-
11-
12-
TO FIX:
13-
- Menu items re-ordering

worksets@blipk.xyz.zip

-646 Bytes
Binary file not shown.

worksets@blipk.xyz/extension.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import * as Main from "resource:///org/gnome/shell/ui/main.js"
5050
// Internal imports
5151
import * as dev from "./dev.js"
5252
import * as sessionManager from "./sessionManager.js"
53+
import * as uiUtils from "./lib/ui/uiUtils.js"
5354

5455
export let WorksetsInstance = null
5556

@@ -121,6 +122,7 @@ export default class Worksets extends Extension {
121122
dev.log( "!~~~~~~~~~~|" )
122123

123124
this.session.saveSession()
125+
uiUtils.clearKnownImages()
124126
if ( this.worksetsIndicator ) this.worksetsIndicator.destroy()
125127
delete this.worksetsIndicator
126128
delete Main.panel.statusArea["WorksetsIndicator"]

worksets@blipk.xyz/lib/ui/shader.js

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,25 +8,25 @@ import { gettext as _ } from "resource:///org/gnome/shell/extensions/extension.j
88
import * as dev from "../../dev.js"
99

1010
// Shader example
11+
const OUTLINE_SHADER_SOURCE = `uniform sampler2D tex;
12+
uniform vec4 v_color = vec4(0, 0, 0, 255);
13+
const vec4 u_outlineColor = vec4(255, 255, 255, 250);
14+
const float smoothing = 1.0/16.0;
15+
const float outlineWidth = 3.0/16.0;
16+
const float outerEdgeCenter = 0.5 - outlineWidth;
17+
18+
void main() {
19+
float distance = texture2D(tex, cogl_tex_coord_in[0].xy).a;
20+
float alpha = smoothstep(outerEdgeCenter - smoothing, outerEdgeCenter + smoothing, distance);
21+
float border = smoothstep(0.5 - smoothing, 0.5 + smoothing, distance);
22+
gl_FragColor = vec4( mix(u_outlineColor.rgb, v_color.rgb, border), alpha );
23+
}`
24+
1125
export var TextOutlineEffect = GObject.registerClass( {
1226
GTypeName: "TextOutlineEffect"
1327
}, class TextOutlineEffect extends Clutter.ShaderEffect {
1428
vfunc_get_static_shader_source() {
15-
try {
16-
return `uniform sampler2D tex;
17-
uniform vec4 v_color = vec4(0, 0, 0, 255);
18-
const vec4 u_outlineColor = vec4(255, 255, 255, 250);
19-
const float smoothing = 1.0/16.0;
20-
const float outlineWidth = 3.0/16.0;
21-
const float outerEdgeCenter = 0.5 - outlineWidth;
22-
23-
void main() {
24-
float distance = texture2D(tex, cogl_tex_coord_in[0].xy).a;
25-
float alpha = smoothstep(outerEdgeCenter - smoothing, outerEdgeCenter + smoothing, distance);
26-
float border = smoothstep(0.5 - smoothing, 0.5 + smoothing, distance);
27-
gl_FragColor = vec4( mix(u_outlineColor.rgb, v_color.rgb, border), alpha );
28-
}`
29-
} catch ( e ) { dev.log( e ) }
29+
return OUTLINE_SHADER_SOURCE
3030
}
3131

3232
vfunc_paint_target( paint_context ) {

worksets@blipk.xyz/lib/ui/uiUtils.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ export function createIconButton( parentItem, iconNames, callback, options, tool
8989
iconButton.leaveEvent] )
9090
parentItem.destroyIconButtons = function () {
9191
parentItem.iconButtons.forEach( function ( iconButton ) {
92-
//iconButton.destroy();
92+
iconButton.destroy?.()
9393
}, this )
9494
parentItem.iconButtons = []
9595
parentItem.iconsButtonsPressIds = []
@@ -190,7 +190,7 @@ export function createTooltip( widget, tooltip ) {
190190
if ( widget.notificationLabel )
191191
removeUserNotification( widget.notificationLabel, 1 )
192192
return false
193-
}
193+
}
194194
)
195195
Me.session.signals.add( widget._tooltipDisappearId )
196196
return false
@@ -220,6 +220,10 @@ export function createTooltip( widget, tooltip ) {
220220
}
221221

222222
export let knownImages = {} // Save on resources generating these in menu refreshes
223+
const KNOWN_IMAGES_MAX = 20
224+
export function clearKnownImages() {
225+
knownImages = {}
226+
}
223227
export function setImage( parent, imgFilePath = "" ) {
224228
try {
225229
let error
@@ -234,7 +238,7 @@ export function setImage( parent, imgFilePath = "" ) {
234238
} else if ( imgFilePath ) {
235239
let pixbuf
236240
try {
237-
pixbuf = GdkPixbuf.Pixbuf.new_from_file( imgFilePath )
241+
pixbuf = GdkPixbuf.Pixbuf.new_from_file_at_scale( imgFilePath, -1, 150, true )
238242
} catch ( e ) {
239243
dev.log( e )
240244
// if ( e instanceof GLib.FileError )
@@ -267,6 +271,9 @@ export function setImage( parent, imgFilePath = "" ) {
267271
if ( !success ) throw Error( "error creating Clutter.Image()" )
268272

269273
knownImages[imgFilePath] = image
274+
const keys = Object.keys( knownImages )
275+
if ( keys.length > KNOWN_IMAGES_MAX )
276+
delete knownImages[keys[0]]
270277
}
271278

272279
// Set the image content on parent (for both cached and newly loaded images)

worksets@blipk.xyz/panelIndicator.js

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -293,9 +293,15 @@ export var WorksetsIndicator = GObject.registerClass( {
293293
"starred-symbolic" : ["non-starred-symbolic", "starred-symbolic"]
294294
let iconOpenNew_nameuri = ( activeIndex > -1 ) ? "window-close-symbolic" : "window-new-symbolic"
295295
let iconOpenHere_nameuri = ( activeIndex > -1 ) ? "view-reveal-symbolic" : "go-jump-symbolic"
296-
let openCloseCommand = ( activeIndex > -1 )
297-
? () => { Me.session.closeWorkset( menuItem.workset ); this._refreshMenu() }
298-
: () => { Me.session.displayWorkset( menuItem.workset, true ); this._refreshMenu() }
296+
let openCloseCommand = () => {
297+
( activeIndex > -1 )
298+
? Me.session.closeWorkset( menuItem.workset )
299+
: Me.session.displayWorkset( menuItem.workset, true )
300+
301+
this._refreshMenu()
302+
Me.workspaceViewManager.refreshOverview()
303+
}
304+
299305
let openCloseMsg = ( activeIndex > -1 )
300306
? "Disengage '" + menuItem.workset.WorksetName + "'"
301307
: "Load '" + menuItem.workset.WorksetName + "' in a new workspace"

0 commit comments

Comments
 (0)