Skip to content

General optimizations #8351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 19, 2019
Merged

General optimizations #8351

merged 5 commits into from
Mar 19, 2019

Conversation

itzexor
Copy link
Contributor

@itzexor itzexor commented Jan 30, 2019

Simple profiling with the GJS/CJS profiler revealed:

  1. paths being called relatively frequently when the features they support weren't enabled:
    -magnifier cursor update
    -desklet mouse tracker
  2. window restack connections are always a top consumer on general non-cinnamon-ui usage
  3. uiGroup is always allocating, and the allocate does nothing beyond default
  4. notification daemon is always checking the WindowTracker to clear notifications, even when there are none

@itzexor itzexor changed the title [WIP] Avoid loading things we're not using [WIP] Avoid loading things we're not using and other optimizations Jan 30, 2019
@itzexor itzexor changed the title [WIP] Avoid loading things we're not using and other optimizations [WIP] General optimizations Feb 2, 2019
@itzexor itzexor closed this Feb 4, 2019
@jaszhix
Copy link
Contributor

jaszhix commented Feb 4, 2019

Just for reference, this was closed after discussions on Slack. I did not intend for this to be closed. There were two problems I pointed out:

  • 885582d switches uiGroup to an StWidget actor, but this loses the skip_paint optimization in CinnamonGenericContainer, and seemed less responsive. I tried to benchmark this with gputest, which might at least show the impact Cinnamon has on the GPU.

Since uiGroup is now entering clutter_actor_paint, before it returns early from the opacity override that skip_paint was replaced with, it places a lock on ClutterContext via _clutter_context_get_pick_mode which calls _clutter_context_get_default. I've seen this affect frame times and have consequently reduced clutter_get_default_backend calls in other PRs because of this.

In addition, this has led me to think allocating in JS with GenericContainer is not the bottleneck, especially if what it's doing is less resource intensive than Clutter's base allocation vfunc.

  • 2dd1532 slows the cursor movement. I'm not sure why. It seems like a muffin or xorg bug which had less impact while Cinnamon was polling XQueryPointer.

@clefebvre clefebvre reopened this Feb 4, 2019
@itzexor
Copy link
Contributor Author

itzexor commented Feb 15, 2019

  • rebased on master
  • dropped uiGroup commit to another branch for future investigation
  • fixed up magnifier commit to cover hopefully all cases where it can be first called.

i think the desklet one definitely wastes resources if you're not using desklets. i'd like to keep that one in if possible.

this timeout calls hasMouseWindow() twice per second, and it was
always running even if there are no enabled desklets.
This avoids adding the zoom actors and connecting to xfixes cursor
changed signal until we're actually called for the first time.
We already do this on every window restack, and that's guaranteed
to catch them all, so we should just store it there and avoid also
calculating it inside _updateRegions.
we already connect in layout.js, so might as well just use that
one to sync the pointer.
If we don't actually have any notification sources we can exit
early and avoid doing anything here.
@itzexor
Copy link
Contributor Author

itzexor commented Feb 18, 2019

  • rebased on master
  • fixed mistake in magnifier.js where I replaced new Object(roi) with Object.Clone(roi) (non existent function) instead of Object.assign({}, roi) as I meant to. This gets rid of a "use an object literal" warning.

@linuxmint linuxmint deleted a comment Feb 18, 2019
@itzexor itzexor dismissed jaszhix’s stale review February 18, 2019 22:32

I think this is resolved.

@itzexor itzexor changed the title [WIP] General optimizations General optimizations Feb 18, 2019
@clefebvre clefebvre merged commit 9b272ce into linuxmint:master Mar 19, 2019
clefebvre pushed a commit that referenced this pull request Jun 5, 2019
* deskletManager: only enable mouse tracking when there are desklets

this timeout calls hasMouseWindow() twice per second, and it was
always running even if there are no enabled desklets.

* magnifier: delay full initialization until magnifier is used

This avoids adding the zoom actors and connecting to xfixes cursor
changed signal until we're actually called for the first time.

* layout.js: only search for popup windows once

We already do this on every window restack, and that's guaranteed
to catch them all, so we should just store it there and avoid also
calculating it inside _updateRegions.

* main/layout: only use one stage::restacked signal connection

we already connect in layout.js, so might as well just use that
one to sync the pointer.

* notificationDaemon: notification autoclear optimization

If we don't actually have any notification sources we can exit
early and avoid doing anything here.
@itzexor itzexor deleted the various-jan-2019 branch July 31, 2019 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants