sugarlabs / sugar

Sugar GTK shell
GNU General Public License v3.0
258 stars 242 forks source link

Fix harmless traceback of sugar-control-panel #802

Closed rhl-bthr closed 5 years ago

rhl-bthr commented 6 years ago

Fixes #793

Tested on: Ubuntu 16.04, Sugar 0.112

quozl commented 6 years ago

Thanks. Reviewed to d153654. I don't like the solution, because it does not explain the problem. A hack. Mysteries should not be left unsolved, because they often come back later. Why is from jarabe.view.buddymenu import BuddyMenu failing, yet the import of BuddyIcon succeeds? Is it because of a recursive import? BuddyMenu is unique in that it appears twice in the traceback.

rhl-bthr commented 6 years ago

I don't like the solution, because it does not explain the problem.

Thanks for pointing out. I have added comments and updated the commit message

Is it because of a recursive import ?

Yes,

quozl commented 6 years ago

Is there an alternative solution? Net effect of a326c5f is to defer the import until the palette is created. At what elapsed time in the user experience? During shell startup before the activity ring is rendered, or when the icon is hovered over or clicked? We have some slow systems that run Sugar, and adding delay in the latter case would be unwelcome.

rhl-bthr commented 6 years ago

I don't have access to any slower system, is there an alternative on how I can test for delay in slower systems on my current system

quozl commented 6 years ago

You can simulate a slower system by configuring a virtual machine hypervisor to restrict the CPU time available. You can then add logger.error calls to critical points in the shell startup. For the OLPC XO, the Python files are pre-compiled (to .pyo files), so you should run one startup test and discard the results before testing again.

However, you may find logger.error calls sufficiently accurate to figure out when your extra delay is incurred.

Remember also that sugar-control-panel is very rarely used, yet favoritesview.py and buddyicon.py are used heavily.

rhl-bthr commented 6 years ago

While I am not getting time to test the delay, I can't see an alternative solution to this

Remember also that sugar-control-panel is very rarely used, yet favoritesview.py and buddyicon.py are used heavily.

I agree. Should we keep this then ?

quozl commented 6 years ago

Perhaps refactor to avoid the recursion?

Look at the traceback again, and consider each import from the top.

Look at extensions/cpsection/frame/model.py; it imports get_view from src/jarabe/frame/__init__.py only in order to reach Frame.settings in frame.py. Nothing else. If Frame.settings was in a separate source file instead of in frame.py, none of the other parts would be imported. The import recursion might be avoided.

What alerted me to this was the PyGIWarning for Gtk and the other libraries. sugar-control-panel doesn't have a GUI. Then no need to add require_version.

The reason there was only one Frame.settings to begin with is that each call to Gio.Settings for the same schema id creates a redundant object, consuming numerous resources, and not delivering signals. See a5358777bf5ffe40988aeaa0d4a69badb4b80634 for details.

chimosky commented 5 years ago

@pro-panda what's the status of this, I hadn't been paying much attention until I recently tried using sugar-control-panel

rhl-bthr commented 5 years ago

@chimosky, thanks for bringing my attention to this. I'm not sure what did you mean by "status".

Stale. Closing