save_tabs function is way too aggressive

[einarjon]

On current master (), the save_tabs function is being called way too much.
When PS1 is set to update the title, save_tabs is called every time Enter is pressed in guake.

To reproduce:
Add this to .bashrc, or just run this snippet in a guake shell.

# If this is an xterm set the title
case "$TERM" in
xterm*|rxvt*)
    PS1="\[\e]0;\W\a\]$PS1"
    ;;
*)
    ;;
esac

Press enter in guake terminal for about a second:
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561134
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json
mom, I've been called: on_terminal_title_changed <function Guake.on_terminal_title_changed at 0x7fd9a0196ea0> at 1566561135
Guake tabs saved to /home/user/.config/guake/session.json

[mlouielu]

We use save_tabs_when_changed decorator to save the tabs.

Currently, it puts on on_new_tab, and on_terminal_title_changed, when the title changed issue by the VTE, it will then save the tabs.

Do you have any idea for another way to save the tabs not that aggressively? By timer for example?

[einarjon]

I'm not sure if a timer is a good idea, but to reduce the load it would make sense to move the decorator to update_window_title

Then on_terminal_title_changed() would need to save the current title, and only call update_window_title() if there is a change.

But it's a real question if the window title really needs to be saved in the restore log.

[aoosapps]

This feature also tends to slow down over time the amount of time it takes for guake to appear when using the keybind. I disabled the saving of tabs and guake went right back to being snappy and responsive.

[mlouielu]

This feature also tends to slow down over time the amount of time it takes for guake to appear when using the keybind. I disabled the saving of tabs and guake went right back to being snappy and responsive.

@isrrapy What is "time it takes for guake to appear", do you mean toggling guake will slow down when enabling this feature?

[ulidtko]

What is "time it takes for guake to appear", do you mean toggling guake will slow down when enabling this feature?

Yes @mlouielu. I noticed added lag for toggle-showing guake in heavy-disk-load situations (like, when compiling a big-ish project).

[einarjon]

Necromancing an old issue since it's still pretty bad.
In the current codebase (commit 00d880f), the tabs are saved when:

1. The title is changed.
2. Starting up (once per saved tab - because of title change)
3. Opening a new tab (twice - once for the new tab and once for title change)
4. Closing a tab (once)

The reason for 2 and the second save in 3. is that the "label" is saved for every tab, .
When a new tab is loaded, the default value is "Terminal", but once the shell loads, the title is set to the current directory, which triggers a call to on_terminal_title_changed(), which has the save_tabs_when_changed decorator.

using while sleep 0.000001; do printf '\x1b]0;%s\x07' "$(date +%s.%N)"; done I can produce and measure about 900 writes/s into session.json from Guake

I think that there is no reason to save the "label" unless when "custom_label_set": true. It will anyway not be used when restoring the tabs. And if it is a custom label, the decorator for on_rename should cover it.
I think we can drop the save_tabs_when_changed decorator from on_terminal_title_changed() without much* loss in functionality. Maybe the "label" should even be skipped (or set to null) when "custom_label_set": false in the JSON.

*) This will miss changes to the "directory" entry, but if guake would just watch for directory changes instead of title changes this would result in fewer saves.

[mlouielu]

See PR #1786, should fix this issue. But please help to test if it raise other issue.

Not sure why Vte.Terminal.get_current_directory_uri() always return None, and therefore Vte.Terminal.signals.current_directory_uri_changed not work, we will need to detect the directory via on_terminal_title_changed.

[ulidtko]

I think that there is no reason to save the "label" unless when "custom_label_set": true. It will anyway not be used when restoring the tabs.

This 👆

[mlouielu]

I think that there is no reason to save the "label" unless when "custom_label_set": true. It will anyway not be used when restoring the tabs.

This point_up_2

This is fixed in #1786, on_terminal_title_changed() will not used the decorator, so it will not do save_tabs().

On the other hand, custom label will save the tabs when on_rename() trigger, so we don't need to rely on update_window_title() to save tabs, too.

[einarjon]

Maybe the "label" should even be skipped (or set to null) when "custom_label_set": false in the JSON.

On the other hand, custom label will save the tabs when on_rename() trigger, so we don't need to rely on update_window_title() to save tabs, too.

That was not what I meant.
The label field stored in the JSON file is never used if "custom_label_set": false , and is overwritten on next startup. So there is no point in storing it in the file.
It would mean minor changes to save_tabs() (skip writing label and custom_label_set if custom_label_set=False) and restore_tabs() (change tab["label"]/tab["custom_label_set"] to tab.get("label", None)/ tab.get("custom_label_set",False)).
That way older session.json files are 100% compatible, but the new, shorter format will break in older versions of guake.