Skip to content
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

g_paste_util_replace: use g_string_replace instead of g_regex_replace_literal #445

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LeoBras
Copy link

@LeoBras LeoBras commented Jun 13, 2024

When using gpaste with storage backup, either selecting a history object for usage or saving a new one will cause the whole history to be converted to xml and stored back to the filesystem.

This conversion is usually fast, but if the user keeps a lot of history saved up, it can take up a couple seconds to convert / save.

When selecting a history object, though, this delay can be noticed by the user, since it takes that much time for the selected object to be available in the clipboard.

When verifying with perf, a lot of time is being taken by g_paste_util_xml_encode, moslty by running g_regex_replace_literal in g_paste_util_replace.

g_paste_util_replace, on the other hand, is only used to replace simple strings. Also, it calls g_regex_escape_string() to make sure all possible regex is not enabled in the replacing pattern, effectively causing it to just replace one string by the other.

In this case, why not replace it directly using g_string_replace instead? It's much faster, considering a history of 27000 itens:

With g_string_replace(), this patch's suggestion:
g_paste_storage_backend_write_history: took 364 miliseconds to run g_paste_storage_backend_write_history: took 364 miliseconds to run g_paste_storage_backend_write_history: took 364 miliseconds to run g_paste_storage_backend_write_history: took 365 miliseconds to run g_paste_storage_backend_write_history: took 362 miliseconds to run

With g_regex_replace_literal():
g_paste_storage_backend_write_history: took 1132 miliseconds to run g_paste_storage_backend_write_history: took 1135 miliseconds to run g_paste_storage_backend_write_history: took 1138 miliseconds to run g_paste_storage_backend_write_history: took 1128 miliseconds to run g_paste_storage_backend_write_history: took 1133 miliseconds to run

Saving almost 70% of cpu usage, and making it much more confortable to the user.

…_literal

When using gpaste with storage backup, either selecting a history object
for usage or saving a new one will cause the whole history to be converted
to xml and stored back to the filesystem.

This conversion is usually fast, but if the user keeps a lot of history
saved up, it can take up a couple seconds to convert / save.

When selecting a history object, though, this delay can be noticed by the
user, since it takes that much time for the selected object to be available
in the clipboard.

When verifying with perf, a lot of time is being taken by
g_paste_util_xml_encode, moslty by running g_regex_replace_literal in
g_paste_util_replace.

g_paste_util_replace, on the other hand, is only used to replace simple
strings. Also, it calls g_regex_escape_string() to make sure all possible
regex is not enabled in the replacing pattern, effectively causing it to
just replace one string by the other.

In this case, why not replace it directly using g_string_replace instead?
It's much faster, considering a history of 27000 itens:

With g_string_replace(), this patch's suggestion:
g_paste_storage_backend_write_history: took 364 miliseconds to run
g_paste_storage_backend_write_history: took 364 miliseconds to run
g_paste_storage_backend_write_history: took 364 miliseconds to run
g_paste_storage_backend_write_history: took 365 miliseconds to run
g_paste_storage_backend_write_history: took 362 miliseconds to run

With g_regex_replace_literal():
g_paste_storage_backend_write_history: took 1132 miliseconds to run
g_paste_storage_backend_write_history: took 1135 miliseconds to run
g_paste_storage_backend_write_history: took 1138 miliseconds to run
g_paste_storage_backend_write_history: took 1128 miliseconds to run
g_paste_storage_backend_write_history: took 1133 miliseconds to run

Saving almost 70% of cpu usage, and making it much more confortable to the
user.

Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
@LeoBras
Copy link
Author

LeoBras commented Jun 13, 2024

While this significantly reduces the latency, having to write all items to disk every time we select / copy something seems overkill.

One alternative is to only save the new item. This can be done like this:

  • Reverse the list before saving, (history = history.prev;)
  • For every new item save it alone at the end of the file (fopen + append)
  • On startup, read in reverse & de-duplicate the items based in uuid
  • Then save the (de-duplicated) list again in disk

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.

1 participant