question concerning timers in lain #114
Comments
|
You are right on I updated the following widgets in order to use unique ids for timer:
but not for
because they're always instantiated once at most. Test 6457c4d and tell me. |
|
Thx for the commits! I am away for a week and will test it when I'm back. |
|
So, not just unique ids, but also the same timeout for one-instance-only widgets, right? Ok, but keep in mind that a user can change timeouts arbitrarily, so the general complexity is not affected. Can you tell me how do you precisely test this? |
|
Plus, please note that |
|
@ff2000 Can you update me on this? Also, tell me how do you do your tests. |
|
The comparison between vicious and lain probably was not that great. As I said I took a similar set of widgets and the same timeouts and compared the resulting CPU usage. Of course the widgets (and the logic behind them) is different. I would need to write my own widgets and adopt them to both APIs. diff --git a/helpers.lua b/helpers.lua
index dbee617..9e0c27f 100644
--- a/helpers.lua
+++ b/helpers.lua
@@ -75,7 +75,8 @@ end
helpers.timer_table = {}
-function helpers.newtimer(name, timeout, fun, nostart)
+function helpers.newtimer(_name, timeout, fun, nostart)
+ local name = timeout
helpers.timer_table[name] = capi.timer({ timeout = timeout })
helpers.timer_table[name]:connect_signal("timeout", fun)
helpers.timer_table[name]:start()(and a change in rc.lua.holo to use the same timeout for all the widgets) |
|
I don't get it. Why name |
|
Damn, sorry :/ Was in a hurry and posted a change I do not actually run. (Fresh git checkout vs. copy in ~/.config/awesome). Here is the fixed up setup: rc_wrap.lua local awesome = require("awesome")
local awful = require("awful")
local lfs = require("lfs")
orig_getdir = awful.util.getdir
awful.util.getdir = function(d)
print(d)
print(awesome.conffile)
print(debug.getinfo(1).source)
print(lfs.currentdir())
if d == "config" then
-- return debug.getinfo(1).source:match("(.*/)")
local path = ""
if awesome.conffile:sub(1,1) == '/' then
path = awesome.conffile
else
path = lfs.currentdir() .. "/" .. awesome.conffile
end
return path:match("(.*/)")
else
return orig_getdir(d)
end
end
dofile(awful.util.getdir("config") .. "/rc.lua.holo.mine")The actual change in helpers.lua: diff --git a/helpers.lua b/helpers.lua
index dbee617..d3f1db5 100644
--- a/helpers.lua
+++ b/helpers.lua
@@ -75,8 +75,11 @@ end
helpers.timer_table = {}
-function helpers.newtimer(name, timeout, fun, nostart)
- helpers.timer_table[name] = capi.timer({ timeout = timeout })
+function helpers.newtimer(_name, timeout, fun, nostart)
+ local name = timeout
+ if not helpers.timer_table[name] then
+ helpers.timer_table[name] = capi.timer({ timeout = timeout })
+ end
helpers.timer_table[name]:connect_signal("timeout", fun)
helpers.timer_table[name]:start()
if not nostart thenAnd the diff to the original rc.lua.holo diff --git a/rc.lua.holo b/rc.lua.holo.mine
index 534dc89..bd35f92 100644
--- a/rc.lua.holo
+++ b/rc.lua.holo.mine
@@ -55,7 +55,7 @@ run_once("unclutter -root")
-- {{{ Variable definitions
-- beautiful init
-beautiful.init(os.getenv("HOME") .. "/.config/awesome/themes/holo/theme.lua")
+beautiful.init(awful.util.getdir("config") .. "/themes/holo/theme.lua")
-- common
modkey = "Mod4"
@@ -106,6 +106,7 @@ mylauncher = awful.widget.launcher({ image = beautiful.awesome_icon,
-- }}}
-- {{{ Wibox
+local ltm = 1
markup = lain.util.markup
blue = "#80CCE6"
space3 = markup.font("Tamsyn 3", " ")
@@ -165,6 +166,7 @@ play_pause_icon = wibox.widget.imagebox()
play_pause_icon:set_image(beautiful.play)
mpdwidget = lain.widgets.mpd({
+ timeout = ltm,
settings = function ()
if mpd_now.state == "play" then
mpd_now.artist = mpd_now.artist:upper():gsub("&.-;", string.lower)
@@ -219,6 +221,7 @@ end)))
-- Battery
batwidget = lain.widgets.bat({
+ timeout = ltm,
settings = function()
bat_header = " Bat "
bat_p = bat_now.perc .. " "
@@ -234,6 +237,7 @@ batwidget = lain.widgets.bat({
-- ALSA volume bar
myvolumebar = lain.widgets.alsabar({
+ timeout = ltm,
card = "0",
ticks = true,
width = 80,
@@ -258,6 +262,7 @@ volumewidget:set_bgimage(beautiful.widget_bg)
-- CPU
cpu_widget = lain.widgets.cpu({
+ timeout = ltm,
settings = function()
widget:set_markup(space3 .. "CPU " .. cpu_now.usage
.. "%" .. markup.font("Tamsyn 5", " "))
@@ -275,6 +280,7 @@ netdown_icon:set_image(beautiful.net_down)
netup_icon = wibox.widget.imagebox()
netup_icon:set_image(beautiful.net_up)
netwidget = lain.widgets.net({
+ timeout = ltm,
settings = function()
widget:set_markup(markup.font("Tamsyn 1", " ") .. net_now.received .. " - "
.. net_now.sent .. space2)And with this setup I get an idle cpu usage (read from the cpu widget) from constant 3%. With the change to helpers.lua reverted it gets up to 4%. But I also see the issue with timeout changes. Might be interesting to change CPU/Disc-intensive widgets to bigger timeouts when the laptop is on battery. So it could really be your approach is better... |
|
I can confirm that the timer sharing patch does indeed improve performance. I can see it in top as well as with the following measurement program: compile: gcc -o ticks ticks.c This makes 3 measurements of each 20 seconds Best used with short timeouts in the widgets and everything else idling. |
|
In order to preserve the possibility to change timers for certain widgets one could add a way so that the user wants to share timers. E.G. a property "use_shared_timers" in lain.helpers table. If set to true helpers.newtimer will index with timeout-id, otherwise it uses the passed arg "name". |
|
I improved a little bit on the test program. I put it here for reference, and in case it is helpful for lain performance testing. I edited my previous comment accordingly. |
I'm a bit rusty in C, after adding ticks.c: In function ‘main’:
ticks.c:25:22: error: storage size of ‘stat1’ isn’t known
struct pstat stat1, stat2;
^
ticks.c:25:29: error: storage size of ‘stat2’ isn’t known
struct pstat stat1, stat2;
^
ticks.c:29:9: warning: implicit declaration of function ‘get_usage’ [-Wimplicit-function-declaration]
get_usage(pid, &stat1);
^
ticks.c:30:9: warning: implicit declaration of function ‘sleep’ [-Wimplicit-function-declaration]
sleep(slp);
^
ticks.c:33:9: warning: implicit declaration of function ‘calc_cpu_usage’ [-Wimplicit-function-declaration]
calc_cpu_usage(&stat2, &stat1, &user_ticks, &system_ticks);
^
ticks.c:34:9: warning: implicit declaration of function ‘calc_cpu_usage_pct’ [-Wimplicit-function-declaration]
calc_cpu_usage_pct(&stat2, &stat1, &user_pct, &system_pct);How do I fix? |
|
You need to combine the two listings I posted. :) To avoid any further confusion, I updated the original comment with my improvements. |
|
what is your conclusion on that? i'd vote for as much timer sharing as On Tue, Sep 15, 2015 at 9:24 AM, Luke Bonham notifications@github.com
|
|
Can you please test 912bd26? It has the proposed patches extended to all widgets. I hope it's ok. |
|
I tried latest master, but I am experiencing a heavier cpu load. This is a current ./ticks $(pgrep awesome) 20 3
Ticks: user = 106 system = 41 sum = 147
Percent: user = 1.3 system = 0.5 sum = 1.8
Ticks: user = 102 system = 37 sum = 139
Percent: user = 1.3 system = 0.5 sum = 1.7
Ticks: user = 111 system = 39 sum = 150
Percent: user = 1.4 system = 0.5 sum = 1.9While this is one with a previous commit: ./ticks $(pgrep awesome) 20 3
Ticks: user = 94 system = 11 sum = 105
Percent: user = 1.2 system = 0.1 sum = 1.3
Ticks: user = 94 system = 10 sum = 104
Percent: user = 1.2 system = 0.1 sum = 1.3
Ticks: user = 104 system = 14 sum = 118
Percent: user = 1.3 system = 0.2 sum = 1.5I don't know if this is related to the fact that I used maps instead of locally instantiating things like @everslick did in it's pull request. I'll investigate further. |
|
Moved to |
|
Testing timer sharing patch: diff --git a/helpers.lua b/helpers.lua
index dbee617..d3f1db5 100644
--- a/helpers.lua
+++ b/helpers.lua
@@ -75,8 +75,11 @@ end
helpers.timer_table = {}
-function helpers.newtimer(name, timeout, fun, nostart)
- helpers.timer_table[name] = capi.timer({ timeout = timeout })
+function helpers.newtimer(_name, timeout, fun, nostart)
+ local name = timeout
+ if not helpers.timer_table[name] then
+ helpers.timer_table[name] = capi.timer({ timeout = timeout })
+ end
helpers.timer_table[name]:connect_signal("timeout", fun)
helpers.timer_table[name]:start()Test 1System: idle With timer sharing $ ./ticks $(pgrep awesome) 20 3
Ticks: user = 43 system = 10 sum = 53
Percent: user = 0.5 system = 0.1 sum = 0.7
Ticks: user = 42 system = 10 sum = 52
Percent: user = 0.5 system = 0.1 sum = 0.7
Ticks: user = 40 system = 8 sum = 48
Percent: user = 0.5 system = 0.1 sum = 0.6Without timer sharing $ ./ticks $(pgrep awesome) 20 3
Ticks: user = 54 system = 7 sum = 61
Percent: user = 0.7 system = 0.1 sum = 0.8
Ticks: user = 55 system = 7 sum = 62
Percent: user = 0.7 system = 0.1 sum = 0.8
Ticks: user = 56 system = 5 sum = 61
Percent: user = 0.7 system = 0.1 sum = 0.8Test 2System: idle With timer sharing $ ./ticks $(pgrep awesome) 20 3
Ticks: user = 31 system = 2 sum = 33
Percent: user = 0.4 system = 0.0 sum = 0.4
Ticks: user = 15 system = 1 sum = 16
Percent: user = 0.2 system = 0.0 sum = 0.2
Ticks: user = 24 system = 4 sum = 28
Percent: user = 0.3 system = 0.1 sum = 0.4Without timer sharing $ ./ticks $(pgrep awesome) 20 3
Ticks: user = 43 system = 11 sum = 54
Percent: user = 0.5 system = 0.1 sum = 0.7
Ticks: user = 48 system = 6 sum = 54
Percent: user = 0.6 system = 0.1 sum = 0.7
Ticks: user = 65 system = 8 sum = 73
Percent: user = 0.8 system = 0.1 sum = 0.9Test 3Using everslick fork at commit 54b6aba Ticks: user = 57 system = 10 sum = 67
Percent: user = 0.7 system = 0.1 sum = 0.8
Ticks: user = 38 system = 13 sum = 51
Percent: user = 0.5 system = 0.2 sum = 0.6
Ticks: user = 38 system = 7 sum = 45
Percent: user = 0.5 system = 0.1 sum = 0.6Test 4Using everslick fork at commit 54b6aba Ticks: user = 20 system = 3 sum = 23
Percent: user = 0.3 system = 0.0 sum = 0.3
Ticks: user = 28 system = 6 sum = 34
Percent: user = 0.4 system = 0.1 sum = 0.4
Ticks: user = 25 system = 4 sum = 29
Percent: user = 0.3 system = 0.1 sum = 0.4ResultI'll use timer sharing with default timeouts. |
helpers.newtimer seems to not use stored timers efficiently. If I add more widgets of the same type (e.g. there are several batteries or sound cards or network devices etc) it creates a new timer and overwrites the previous one in the table (as I undertand LUA - which might be wrong). vicious indexes the timer table with the actual timeout which really seems to greatly affect wakeups and general CPU usage (0-1% in contrast to 2-3% with lain, both running a comparable set of widgets)
Furthermore vicious seems to efficiently cache formatted strings (don't get that part of the code entirely ATM, will have to stare at it some more time ;))
I think lain could take a similar approach to maximize performance/energy efficiency.
The text was updated successfully, but these errors were encountered: