pixunil / cinnamon-applet-system-monitor

An applet for Cinnamon which shows CPU, Memory and Swap usage, Disk and Network rates with graphs
http://cinnamon-spices.linuxmint.com/applets/view/198
GNU General Public License v2.0
15 stars 5 forks source link

The applet causes screen freezes #1

Closed VladV closed 10 years ago

VladV commented 10 years ago

Hello.

I encountered a strange issue which seems to be caused by your applet. Just before the applet data is refreshed, almost everything on the screen freezes for a moment. The mouse cursor moves freely, but anything more complex - a video playing, a page scrolling etc. - stops for a moment. Here is a video that demonstrates the problem with glxgears: https://dl.dropboxusercontent.com/u/644287/system-monitor-pixunil-lags.ogv.

It took me a while to track the problem down to the applet (the first thoughts were of video driver or even kernel bugs), but now I am pretty sure it is the cause. Here are a few things I tried: 1) remove the applet from the panel, then log out and in again - the problem goes away 2) change sampling interval in the applet settings - interval between the freezes changes accordingly 3) create a new user and install this single applet - no problem with the clean profile, freezes appear after the applet is installed

The issue is there both in the release version, and in the current master on GitHub.

My system: Linux Mint 17 Qiana 64bit (cinnamon) Linux 3.13.0-24-generic #47-Ubuntu SMP Fri May 2 23:30:00 UTC 2014 x86_64 Cinnamon 2.2.16 (from Qiana repos) Radeon HD 4670 (radeon driver)

pixunil commented 10 years ago

Thanks for the report, VladV. I would like to see the output of sensors in a terminal, because this is what exactly the applet does to get the thermal data, and your thermal is at NaN°C (Not a Number). I hope that this is the problem, but this wouldn't explain your third point.

VladV commented 10 years ago

I believe, NaN is shown because fan data is processed incorrectly.

$ sensors
radeon-pci-0100
Adapter: PCI adapter
temp1:        +41.0°C  (crit = +120.0°C, hyst = +90.0°C)

coretemp-isa-0000
Adapter: ISA adapter
Core 0:       +40.0°C  (high = +83.0°C, crit = +99.0°C)
Core 1:       +38.0°C  (high = +83.0°C, crit = +99.0°C)
Core 2:       +38.0°C  (high = +83.0°C, crit = +99.0°C)
Core 3:       +38.0°C  (high = +83.0°C, crit = +99.0°C)

atk0110-acpi-0
Adapter: ACPI interface
Vcore Voltage:       +1.23 V  (min =  +0.80 V, max =  +1.60 V)
+3.3V Voltage:       +3.31 V  (min =  +2.97 V, max =  +3.63 V)
+5V Voltage:         +5.06 V  (min =  +4.50 V, max =  +5.50 V)
+12V Voltage:       +12.32 V  (min = +10.20 V, max = +13.80 V)
CPU Fan Speed:       740 RPM  (min =  600 RPM, max = 7200 RPM)
Chassis1 Fan Speed:    0 RPM  (min =  600 RPM, max = 7200 RPM)
Chassis2 Fan Speed: 1936 RPM  (min =  600 RPM, max = 7200 RPM)
Power Fan Speed:       0 RPM  (min =    0 RPM, max = 7200 RPM)
CPU Temperature:     +39.5°C  (high = +45.0°C, crit = +45.5°C)
MB Temperature:      +28.0°C  (high = +45.0°C, crit = +46.0°C)
pixunil commented 10 years ago

I updated the thermal modul of the applet, but I think this was not all work. Do you have other issues, like disk rates not working? Or is one of the sub menus really full? I don't think this is a performance thing, because you have really good hardware.

VladV commented 10 years ago

Thanks, the temperature is showing fine now. Maybe you would consider adding fan speeds as a separate block?

However, the freezes are still there.

Disk rates are not working indeed, zero is shown. Please see the screenshot: https://dl.dropboxusercontent.com/u/644287/system-monitor-pixunil-screen.png

It might be important that my home and root partitions are on a software RAID (Linux native, configured with mdadm). When I tried running dd on another disk without RAID, the applet showed it (though the applet was showing read speed, whereas I was writing to the disk).

pixunil commented 10 years ago

Yes, this is kinda strange with GTop (or my applet) with the disk rates...

Can you please do the following steps:

  1. download the applet.js from the bugtracking branch into the applet dir
  2. restart cinnamon
  3. wait ~5 seconds
  4. open Looking Glass with Alt+F2 and lg[Enter]
  5. screenshot

This is just for looking which task drains your performance.

VladV commented 10 years ago

The problem with disk rates is probably not in your applet. I tried a few other monitoring applets, some of them show rates for physical drives (like /dev/sdb) when data is written to the raid partition, but I haven't seen any applet showing I/O for the raid partitions (like /dev/md1). I think, it might not be possible to get I/O rate for a partition at all.

Here is the screenshot: https://dl.dropboxusercontent.com/u/644287/system-monitor-pixunil-lg.png

pixunil commented 10 years ago

Okay, than it's GTop's fault. Thanks for your screenshot, but I'm not getting smarter... You have better scores than I (no surprise for me), only the thermal value is higher than my.

Two things you can check for me:

VladV commented 10 years ago

Yes, fixed in both cases. Thank you!

Another interesting thing, if I just run sensors in the terminal (watch -n 1 sensors), no freezes occur. As far as I understand, your applet does just that, so it must be an issue of interaction between the applet and the process it starts.

May I suggest using libsensor instead of starting a separate process? That might improve the performance significantly.

I also tried running psensor, it seems to use libsensor and causes no freezes. Though it might not be a fair comparison, since psensor is a standalone app, not an applet.

pixunil commented 10 years ago

Okay, than I have a late question: Does this bug appears immediately after installing the applet, or after what "event"? I hope there is support for libsensor in gjs / cjs…

VladV commented 10 years ago

I checked it again, the bug appears immediately.

pixunil commented 10 years ago

Another performance test is in the bugtracking branch, this time only for thermal. Please send me a screenshot of the Looking Glass error log.

VladV commented 10 years ago

https://dl.dropboxusercontent.com/u/644287/system-monitor-pixunil-lg1.png

pixunil commented 10 years ago

Thanks, that helped me really. It really seems to be the commandline, but a libsensor module for js is sadly not available. If you now download the source in the bugtracking branch, I made a little workaround. Tell me if it's working fine now.

VladV commented 10 years ago

Unfortunately, it's not. The freezes still happen whenever sensors process is started (I watched it with pstree to be sure).

pixunil commented 10 years ago

Okay, now I'm stuck. No libsensors and the way I called sensors has no alternatives. Can you please install the CPU Temperature Indicator applet, because this has a nearly same way to get the thermal values. Are the freezes there to?

VladV commented 10 years ago

Yes, the freezes are there as well. I found a discussion here: https://github.com/linuxmint/Cinnamon/issues/2311

pixunil commented 10 years ago

Thanks, I'll take a view. I won't solve the problem in the next time, but I will add a option to disable thermal.

ghost commented 10 years ago

Hi. You can not do an spaw command line Sync or Cinnamon will be freezed waiting for the result of the terminal. This was for me a big problem for some time, but i found a solution and i implemented an spaw command line Async.... Here, I pass the source code of a Terminal Reader class, that can read async the terminal, so can be used without a cinnamon freezes...

I'm happy to help....

@pixunil I can not underestand what you do here: https://github.com/linuxmint/Cinnamon/pull/3566 What we could be resolved with the regular expressions?

I also want to say that i try to provide a cinnamon installer with the ability to check dependencies, like

Ubuntu: gir1.2-gtop
Fedora: libgtop2-devel
Arch: libgtop

and install this automatically, depending of the current distro... The project it's a missed now, but i try to improve the code, to be more clean and faster. I also want to add more things and integrating of the Cinnamon Installer in the cinnamon settings... Actually this could be done, adding a cs_module to cinnamon settings, but witch this way we duplicate the current data.

Here you can find my ideas about what i want to do: https://github.com/linuxmint/Cinnamon/issues/3562

Here you can find the Cinnamon Installer project: https://github.com/lestcape/Cinnamon-Installer

I want ideas of how will be the best way to add dependencies to packages to metadata.json.

Any help will be appreciated.

ghost commented 10 years ago

Usage (with the terminal instance you can also cancel the current read proccess ):

   trySpawnAsyncPipe: function(command, callback) {
      let terminal = new TerminalReader(command, callback);
      terminal.executeReader();
      return terminal;
   }

Class Terminal Reader

function TerminalReader(command, callback) {
   this._init(command, callback);
}

TerminalReader.prototype = {
   _init: function(command, callback) {
      this._callbackPipe = callback;
      this._commandPipe = command;
      this.idle = true;
      this._childWatch = null;
   },

   executeReader: function() {
      if(this.idle) {
         this.idle = false;
         try {
            let [success, argv] = GLib.shell_parse_argv("sh -c '" + this._commandPipe + "'");
            if(success) {
               let [exit, pid, stdin, stdout, stderr] =
                    GLib.spawn_async_with_pipes(null,
                                                argv, 
                                                null,
                                                GLib.SpawnFlags.SEARCH_PATH | GLib.SpawnFlags.DO_NOT_REAP_CHILD, 
                                                null );

               this._childPid = pid;
               this._stdin = new Gio.UnixOutputStream({ fd: stdin, close_fd: true });
               this._stdout = new Gio.UnixInputStream({ fd: stdout, close_fd: true });
               this._stderr = new Gio.UnixInputStream({ fd: stderr, close_fd: true });

               // We need this one too, even if don't actually care of what the process
               // has to say on stderr, because otherwise the fd opened by g_spawn_async_with_pipes
               // is kept open indefinitely
               this._stderrStream = new Gio.DataInputStream({ base_stream: this._stderr });
               this._dataStdout = new Gio.DataInputStream({ base_stream: this._stdout });
               this._cancellableStderrStream = new Gio.Cancellable();
               this._cancellableStdout = new Gio.Cancellable();

               this.resOut = 1;
               this._readStdout();
               this.resErr = 1;
               this._readStderror();

               this._childWatch = GLib.child_watch_add(GLib.PRIORITY_DEFAULT, pid, Lang.bind(this, function(pid, status, requestObj) {
                  GLib.source_remove(this._childWatch);
                  this._childWatch = null;
                  this._stdin.close(null);
                  this.idle = true;
               }));
            }
            //throw
         } catch(err) {
            if (err.code == GLib.SpawnError.G_SPAWN_ERROR_NOENT) {
               err.message = _("Command not found.");
            } else {
               // The exception from gjs contains an error string like:
               //   Error invoking GLib.spawn_command_line_async: Failed to
               //   execute child process "foo" (No such file or directory)
               // We are only interested in the part in the parentheses. (And
               // we can't pattern match the text, since it gets localized.)
               err.message = err.message.replace(/.*\((.+)\)/, '$1');
            }
            throw err;
         }
      }
   },

   destroy: function() {
      try {
         if(this._childWatch) {
            GLib.source_remove(this._childWatch);
            this._childWatch = null;
         }
         if(!this._dataStdout.is_closed()) {
            this._cancellableStdout.cancel();
            this._stdout.close_async(0, null, Lang.bind(this, this.closeStdout));
         }
         if(!this._stderrStream.is_closed()) {
            this._cancellableStderrStream.cancel();
            this._stderrStream.close_async(0, null, Lang.bind(this, this.closeStderrStream));
         }
         this._stdin.close(null);
         this.idle = true;
      }
      catch(e) {
         Main.notify("Error on close" + this._dataStdout.is_closed(), e.message);
      }
   },

   closeStderrStream: function(std, result) {
      try {
        std.close_finish(result);
      } catch(e) {
         std.close_async(0, null, Lang.bind(this, this.closeStderrStream));
      }
   },

   closeStdout: function(std, result) {
      try {
        std.close_finish(result);
      } catch(e) {
         std.close_async(0, null, Lang.bind(this, this.closeStderrStream));
      }
   },

   _readStdout: function() {
      this._dataStdout.fill_async(-1, GLib.PRIORITY_DEFAULT, this._cancellableStdout, Lang.bind(this, function(stream, result) {
         try {
            if(!this._dataStdout.is_closed()) {
               if(this.resOut != -1)
                  this.resOut = this._dataStdout.fill_finish(result);// end of file
               if(this.resOut == 0) {
                  let val = stream.peek_buffer().toString();
                  if(val != "")
                     this._callbackPipe(this._commandPipe, true, val);
                  this._stdout.close(this._cancellableStdout);
               } else {
                  // Try to read more
                  this._dataStdout.set_buffer_size(2 * this._dataStdout.get_buffer_size());
                  this._readStdout();
               }
            }
         } catch(e) {
            global.log(e);
         }
      }));
   },

   _readStderror: function() {
      this._stderrStream.fill_async(-1, GLib.PRIORITY_DEFAULT, this._cancellableStderrStream, Lang.bind(this, function(stream, result) {
         try {
            if(!this._stderrStream.is_closed()) {
               if(this.resErr != -1)
                  this.resErr = this._stderrStream.fill_finish(result);
               if(this.resErr == 0) { // end of file
                  let val = stream.peek_buffer().toString();
                  if(val != "")
                     this._callbackPipe(this._commandPipe, false, val);
                  this._stderr.close(null);
               } else {
                  this._stderrStream.set_buffer_size(2 * this._stderrStream.get_buffer_size());
                  this._readStderror();
               }
            }
         } catch(e) {
            global.log(e);
         }
      }));
   }
};

function Updater() {
   this._init();
}

Updater.prototype = {
   _init: function() {
      this.pythonVer = "python3";
      this.terminal = new Terminal();
      this.pathToLocalUpdater = this.terminal.getHomeRelativeFile(".local/share/cinnamon/applets/" + UUID + "/pkg_updater/Updater.py");
      this.pathToRemoteUpdater = this.terminal.getHomeRelativeFile(".local/share/Cinnamon-Installer/Cinnamon-Installer/Updater.py");
   },

   getUpdaterPath: function() {
      let updaterPath = "";
      if(this.terminal.fileExist(this.pathToLocalUpdater))
         updaterPath = this.pathToLocalUpdater;
      else if(this.terminal.fileExist(this.pathToRemoteUpdater))
         updaterPath = this.pathToRemoteUpdater;
      if(this.terminal.allowFileExecution(updaterPath))
         return updaterPath;
      return "";
   },

//is file: GLib.FileTest.IS_REGULAR
//is dir: GLib.FileTest.IS_DIR
   executeUpdater: function(action, mode) {
      let updaterPath = this.getUpdaterPath();
      if(updaterPath != "") {
         if(action == "update")
            action = "--qupdate";
         else if(action == "uninstall")
            action = "--uninstall";
         let query = this.pythonVer + " " + updaterPath + " " + action + " " + mode;
         if((action == "--qupdate")&&(mode == "silent"))
             this.terminal.execCommandSyncPipe(query, Lang.bind(this, this.update));
         else
             this.terminal.execCommand(query);
      }
   },

   update: function(command, sucess, result) {
      //"update" and "ready"
      if(result.indexOf("update") == 0) {
         this.executeUpdater("update", "gui");
      } else if(result.indexOf("internet") == 0) {
         Main.notify(_("Internet connection is required to check for update of Cinnamon Installer."));
      }
   }
};
ghost commented 10 years ago

I have also some wrapper to check the correctly spaw signature:

execCommand: function(command) {
      try {
         let [success, argv] = GLib.shell_parse_argv(command);
         this.trySpawnAsync(argv);
         return true;
      } catch (e) {
         let title = _("Execution of '%s' failed:").format(command);
         Main.notifyError(title, e.message);
      }
      return false;
   },

   execCommandSyncPipe: function(command, callBackFunction) {
      try {
         return this.trySpawnAsyncPipe(command, callBackFunction);
      } catch (e) {
         let title = _("Execution of '%s' failed:").format(command);
         Main.notifyError(title, e.message);
      }
      return null;
   },
ghost commented 10 years ago

The signature of callback function:

callback: function(command, sucess, result) {
   .... do some thing..........
}

A call:

this.terminal.execCommandSyncPipe(query, Lang.bind(this, this.callback));
ghost commented 10 years ago

@pixunil l see what can I do with your changes in xlet. I really want a dynamically settings, and the change help to do that. https://github.com/linuxmint/Cinnamon/issues/3532

pixunil commented 10 years ago

Thanks @lestcape, is it planned to push it to the imports of cjs? It is much code, but I'll implement it.

pixunil commented 10 years ago

@VladV, can you please pull the new commit and look if this issue still exists?

ghost commented 10 years ago

@pixunil You don' t need the class Updater, it's an example an it's part of the client of Cinnamon Installer, using in Configurable Menu, you only need the terminal reader...

I don't know if could be possible, merge this code inside cinnamon, because normally they don't want to provide things for external devs... They only merge things that they can used... Will be for me really usefull, but well... As @VladV say see: https://github.com/linuxmint/Cinnamon/issues/2311 When i try to report the problem, they close my thread, so especially in this case i don't want to provide this for cinnamon, but you can if you want.

VladV commented 10 years ago

In the latest version, the freezes are gone. Big thanks to both of you!

pixunil commented 10 years ago

Okay, this will go also into the new version 1.2 (will be released next week)