Hi,
I started using Geany a couple of weeks ago and I noticed a slower response to typed text than my previous editor Scite. Running both editors at the same time, it was noticeable to me that Geany had a slightly longer latency time for printing the characters on the screen (and/or for moving the cursor to the next line after hitting the Enter key).
Well, I think that might not be noticeable to most of users (since they run Geany on fast machines) but It is annoying me because I'm working on a shared server (which most of time is very loaded), making the latency even bigger. So, I've been trying to identify the cause of the latency.
After some experiments and I think I could find the cause and an workaround. I just need some help from you guys for getting a final solution.
I see that the latency is caused the following sequence of function calls:
editor.c: on_editor_notify() inside "switch (nt->nmhdr.code) ... case SCN_MODIFIED" : document_undo_add(doc, UNDO_SCINTILLA, NULL); (file: document.c) document_set_text_changed(doc, TRUE); (file: document.c) ui_update_tab_status(doc) (file: ui_utils.c) sidebar_openfiles_update(doc); (file: ui_utils.c)
It seems that the latency is because sidebar_openfiles_update() is called too frequently (apparently at each typed key). I've made an experiment by commenting out the call to ui_update_tab_status(doc) (insided document_set_text_changed() ) and I got Geany as sharp as Scite!!! :-)
I'd like to propose the fix below (please find the patch in the end of this message). It worked fine for me but since I'm not much familiar with Geany source code, I need you guys to review it.
Could you guys please review my fix? Does any of you see any better solution?
Please notice that I'm changing two functions: document_undo_add() - reduced the latency while typing text document_redo_add() - reduced the latency for the undo operation (CTRL+Z)
Thanks in advance and regards,
Evandro
diff -Naurp geany-0.20/src/document.c geany-0.20_fast/src/document.c --- geany-0.20/src/document.c 2012-11-23 20:22:22.564735000 -0800 +++ geany-0.20_fast/src/document.c 2012-11-23 20:28:17.535008000 -0800 @@ -2676,7 +2676,9 @@ void document_undo_add(GeanyDocument *do
g_trash_stack_push(&doc->priv->undo_actions, action);
- document_set_text_changed(doc, TRUE); + /* document_set_text_changed(doc, TRUE); */ + doc->changed = TRUE; + ui_update_popup_reundo_items(doc); }
@@ -2840,7 +2842,9 @@ static void document_redo_add(GeanyDocum
g_trash_stack_push(&doc->priv->redo_actions, action);
- document_set_text_changed(doc, TRUE); + /* document_set_text_changed(doc, TRUE); */ + doc->changed = TRUE; + ui_update_popup_reundo_items(doc); }
On 24 November 2012 16:15, Evandro Borracini evandro.borracini@gmail.com wrote:
Hi,
I started using Geany a couple of weeks ago and I noticed a slower response to typed text than my previous editor Scite. Running both editors at the same time, it was noticeable to me that Geany had a slightly longer latency time for printing the characters on the screen (and/or for moving the cursor to the next line after hitting the Enter key).
Well, I think that might not be noticeable to most of users (since they run Geany on fast machines) but It is annoying me because I'm working on a shared server (which most of time is very loaded), making the latency even bigger. So, I've been trying to identify the cause of the latency.
After some experiments and I think I could find the cause and an workaround. I just need some help from you guys for getting a final solution.
I see that the latency is caused the following sequence of function calls:
editor.c: on_editor_notify() inside "switch (nt->nmhdr.code) ... case SCN_MODIFIED" : document_undo_add(doc, UNDO_SCINTILLA, NULL); (file: document.c) document_set_text_changed(doc, TRUE); (file: document.c) ui_update_tab_status(doc) (file: ui_utils.c) sidebar_openfiles_update(doc); (file: ui_utils.c)
It seems that the latency is because sidebar_openfiles_update() is called too frequently (apparently at each typed key). I've made an experiment by commenting out the call to ui_update_tab_status(doc) (insided document_set_text_changed() ) and I got Geany as sharp as Scite!!! :-)
I'd like to propose the fix below (please find the patch in the end of this message). It worked fine for me but since I'm not much familiar with Geany source code, I need you guys to review it.
Could you guys please review my fix? Does any of you see any better solution?
Please notice that I'm changing two functions: document_undo_add() - reduced the latency while typing text document_redo_add() - reduced the latency for the undo operation (CTRL+Z)
Thanks in advance and regards,
Evandro
diff -Naurp geany-0.20/src/document.c geany-0.20_fast/src/document.c --- geany-0.20/src/document.c 2012-11-23 20:22:22.564735000 -0800 +++ geany-0.20_fast/src/document.c 2012-11-23 20:28:17.535008000 -0800 @@ -2676,7 +2676,9 @@ void document_undo_add(GeanyDocument *do
g_trash_stack_push(&doc->priv->undo_actions, action);
- document_set_text_changed(doc, TRUE);
- /* document_set_text_changed(doc, TRUE); */
Hi,
I don't think removing a lot of UI update calls is the right solution unless you can show that they are done elsewhere. The things that are done by these calls must be done somewhere to make the UI respond correctly to document changed status.
A possible better solution (that I have not tested) is for document_set_text_changed() to only call the UI updates if the changed status is changed.
Cheers Lex
- doc->changed = TRUE;
- ui_update_popup_reundo_items(doc);
}
@@ -2840,7 +2842,9 @@ static void document_redo_add(GeanyDocum
g_trash_stack_push(&doc->priv->redo_actions, action);
- document_set_text_changed(doc, TRUE);
- /* document_set_text_changed(doc, TRUE); */
- doc->changed = TRUE;
- ui_update_popup_reundo_items(doc);
}
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Hi Lex,
Thanks for your reply. In the original code, document_redo/undo_add() calls document_set_text_changed(doc, TRUE) so it's hardcoded with changed=TRUE and it would always call the ui updates even if we added an if (changed == TRUE)...
void document_set_text_changed(GeanyDocument *doc, gboolean changed) { g_return_if_fail(doc != NULL);
doc->changed = changed;
if (! main_status.quitting) { ui_update_tab_status(doc); ui_save_buttons_toggle(changed); ui_set_window_title(doc); ui_update_statusbar(doc, -1); } }
Interestingly, is that I still see updates in the sidebar, buttons, windows title and status bar after commenting out the call to document_set_text_changed() inside document_redo/undo_add(). So, I think it's called elsewhere.
I'll study the code a bit more to try to understand how the ui updates works.
Another interesting point is that only ui_update_tab_status(doc) creates a noticeable latency. The other functions: ui_save_buttons_toggle(changed), ui_set_window_title(doc), ui_update_statusbar(doc, -1) seem to be fast.
Thanks again and regards,
Evandro
2012/11/24 Lex Trotman elextr@gmail.com
On 24 November 2012 16:15, Evandro Borracini evandro.borracini@gmail.com wrote:
Hi,
I started using Geany a couple of weeks ago and I noticed a slower
response
to typed text than my previous editor Scite. Running both editors at the same time, it was noticeable to me that Geany had a slightly longer
latency
time for printing the characters on the screen (and/or for moving the
cursor
to the next line after hitting the Enter key).
Well, I think that might not be noticeable to most of users (since they
run
Geany on fast machines) but It is annoying me because I'm working on a shared server (which most of time is very loaded), making the latency
even
bigger. So, I've been trying to identify the cause of the latency.
After some experiments and I think I could find the cause and an
workaround.
I just need some help from you guys for getting a final solution.
I see that the latency is caused the following sequence of function
calls:
editor.c: on_editor_notify() inside "switch (nt->nmhdr.code) ... case SCN_MODIFIED" : document_undo_add(doc, UNDO_SCINTILLA, NULL); (file: document.c) document_set_text_changed(doc, TRUE); (file: document.c) ui_update_tab_status(doc) (file: ui_utils.c) sidebar_openfiles_update(doc); (file: ui_utils.c)
It seems that the latency is because sidebar_openfiles_update() is called too frequently (apparently at each typed key). I've made an experiment by commenting out the call to ui_update_tab_status(doc) (insided document_set_text_changed() ) and I got Geany as sharp as Scite!!! :-)
I'd like to propose the fix below (please find the patch in the end of
this
message). It worked fine for me but since I'm not much familiar with
Geany
source code, I need you guys to review it.
Could you guys please review my fix? Does any of you see any better solution?
Please notice that I'm changing two functions: document_undo_add() - reduced the latency while typing text document_redo_add() - reduced the latency for the undo operation (CTRL+Z)
Thanks in advance and regards,
Evandro
diff -Naurp geany-0.20/src/document.c geany-0.20_fast/src/document.c --- geany-0.20/src/document.c 2012-11-23 20:22:22.564735000 -0800 +++ geany-0.20_fast/src/document.c 2012-11-23 20:28:17.535008000 -0800 @@ -2676,7 +2676,9 @@ void document_undo_add(GeanyDocument *do
g_trash_stack_push(&doc->priv->undo_actions, action);
- document_set_text_changed(doc, TRUE);
- /* document_set_text_changed(doc, TRUE); */
Hi,
I don't think removing a lot of UI update calls is the right solution unless you can show that they are done elsewhere. The things that are done by these calls must be done somewhere to make the UI respond correctly to document changed status.
A possible better solution (that I have not tested) is for document_set_text_changed() to only call the UI updates if the changed status is changed.
Cheers Lex
- doc->changed = TRUE;
- ui_update_popup_reundo_items(doc);
}
@@ -2840,7 +2842,9 @@ static void document_redo_add(GeanyDocum
g_trash_stack_push(&doc->priv->redo_actions, action);
- document_set_text_changed(doc, TRUE);
- /* document_set_text_changed(doc, TRUE); */
- doc->changed = TRUE;
- ui_update_popup_reundo_items(doc);
}
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Le 24/11/2012 14:48, Evandro Borracini a écrit :
Hi Lex,
Thanks for your reply. In the original code, document_redo/undo_add() calls document_set_text_changed(doc, TRUE) so it's hardcoded with changed=TRUE and it would always call the ui updates even if we added an if (changed == TRUE)...
void document_set_text_changed(GeanyDocument *doc, gboolean changed) { g_return_if_fail(doc != NULL);
doc->changed = changed; if (! main_status.quitting) { ui_update_tab_status(doc); ui_save_buttons_toggle(changed); ui_set_window_title(doc); ui_update_statusbar(doc, -1); }}
A solution might be to only do the updates if (doc->changed != changed). I didn't look at the interactions with the rest of Geany (nor test it actually), but looking at this I guess that changing it to:
void document_set_text_changed(GeanyDocument *doc, gboolean changed) { g_return_if_fail(doc != NULL);
if (doc->changed != changed && ! main_status.quitting) { doc->changed = changed; /* maybe this should also be called if * (main_status.quitting), but I'm not * sure it's necessary */ ui_update_tab_status(doc); ui_save_buttons_toggle(changed); ui_set_window_title(doc); ui_update_statusbar(doc, -1); } }
would work and fix the issue without changing the code much.
Cheers, Colomban
Interestingly, is that I still see updates in the sidebar, buttons, windows title and status bar after commenting out the call to document_set_text_changed() inside document_redo/undo_add(). So, I think it's called elsewhere.
I'll study the code a bit more to try to understand how the ui updates works.
Another interesting point is that only ui_update_tab_status(doc) creates a noticeable latency. The other functions: ui_save_buttons_toggle(changed), ui_set_window_title(doc), ui_update_statusbar(doc, -1) seem to be fast.
On 24/11/2012 14:04, Colomban Wendling wrote:
A solution might be to only do the updates if (doc->changed != changed). I didn't look at the interactions with the rest of Geany (nor test it actually), but looking at this I guess that changing it to:
I played with this a little. It won't work easily because document_set_text_changed is sometimes called to force a UI update, possibly even when the current doc hasn't changed. The API docs describe this behavior also.
Hi,
Thanks everyone for the comments and suggestions. Based on them, I reverted back to the original code and just added an "if (doc->changed != TRUE)" before calling document_set_text_changed(doc, TRUE) in document_undo/redo_add().
That prevents calling document_set_text_changed() again if doc->changed is already TRUE.
It worked fine and I don't see the latency anymore. :-)
Please find the complete patch below. Document_set_text_changed() function is not changed.
Please comment on that.
Thanks again and regards,
Evandro
2012/11/24 Nick Treleaven nick.treleaven@btinternet.com
On 24/11/2012 14:04, Colomban Wendling wrote:
A solution might be to only do the updates if (doc->changed != changed). I didn't look at the interactions with the rest of Geany (nor test it actually), but looking at this I guess that changing it to:
I played with this a little. It won't work easily because document_set_text_changed is sometimes called to force a UI update, possibly even when the current doc hasn't changed. The API docs describe this behavior also.
______________________________**_________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-**bin/mailman/listinfo/develhttps://lists.geany.org/cgi-bin/mailman/listinfo/devel
Sorry, I forgot to include the patch.
Thanks,
Evandro
diff -Naurp geany-0.20/src/document.c geany-0.20_fast/src/document.c --- geany-0.20/src/document.c 2012-11-23 20:22:22.564735000 -0800 +++ geany-0.20_fast/src/document.c 2012-11-24 12:47:50.110927000 -0800 @@ -2676,7 +2676,10 @@ void document_undo_add(GeanyDocument *do
g_trash_stack_push(&doc->priv->undo_actions, action);
- document_set_text_changed(doc, TRUE); + if (doc->changed != TRUE) { + document_set_text_changed(doc, TRUE); + } + ui_update_popup_reundo_items(doc); }
@@ -2840,7 +2843,10 @@ static void document_redo_add(GeanyDocum
g_trash_stack_push(&doc->priv->redo_actions, action);
- document_set_text_changed(doc, TRUE); + if (doc->changed != TRUE) { + document_set_text_changed(doc, TRUE); + } + ui_update_popup_reundo_items(doc); }
2012/11/24 Evandro Borracini evandro.borracini@gmail.com
Hi,
Thanks everyone for the comments and suggestions. Based on them, I reverted back to the original code and just added an "if (doc->changed != TRUE)" before calling document_set_text_changed(doc, TRUE) in document_undo/redo_add().
That prevents calling document_set_text_changed() again if doc->changed is already TRUE.
It worked fine and I don't see the latency anymore. :-)
Please find the complete patch below. Document_set_text_changed() function is not changed.
Please comment on that.
Thanks again and regards,
Evandro
2012/11/24 Nick Treleaven nick.treleaven@btinternet.com
On 24/11/2012 14:04, Colomban Wendling wrote:
A solution might be to only do the updates if (doc->changed != changed). I didn't look at the interactions with the rest of Geany (nor test it actually), but looking at this I guess that changing it to:
I played with this a little. It won't work easily because document_set_text_changed is sometimes called to force a UI update, possibly even when the current doc hasn't changed. The API docs describe this behavior also.
______________________________**_________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-**bin/mailman/listinfo/develhttps://lists.geany.org/cgi-bin/mailman/listinfo/devel
On Sat, 24 Nov 2012 14:09:12 +0000 Nick Treleaven nick.treleaven@btinternet.com wrote:
I played with this a little. It won't work easily because document_set_text_changed is sometimes called to force a UI update, possibly even when the current doc hasn't changed. [...]
On Sat, 24 Nov 2012 15:11:48 -0600 Evandro Borracini evandro.borracini@gmail.com wrote:
Thanks everyone for the comments and suggestions. Based on them, I reverted back to the original code and just added an "if (doc->changed != TRUE)" before calling document_set_text_changed (doc, TRUE) in document_undo/redo_add().
document_redo_add(doc, UNDO_SCINTILLA, NULL); <-- text changed document_redo_add(doc, UNDO_BOM, GINT_TO_POINTER(doc->has_bom)); document_redo_add(doc, UNDO_ENCODING, g_strdup(doc->encoding));
The 2nd and 3rd call probably rely on redo_add to call document_set_text_changed(), to update the status bar. You can check that (my Geany version is a bit outdated), and update the patch to call set_text for BOM/ENCODING (they are rare). Same for undo.
Aside from that, the patch looks reasonable. For plugins, we allow document_set_text_changed(), but not undo/redo_add(), which is OK.
Thanks Dimitar,
I've updated the patch to always call document_set_text_changed() for BOM/ENCODING. For UNDO_SCINTILLA, document_set_text_changed() is called only at the first document change.
I've tested and it worked fine for me. The latency while typing is gone. As expected, we still have a small latency at the first key hit but that doesn't create any trouble at all.
The new patch is below.
Best Regards,
Evandro
diff -Naurp geany-0.20/src/document.c geany-0.20_fast/src/document.c --- geany-0.20/src/document.c 2012-11-23 20:22:22.564735000 -0800 +++ geany-0.20_fast/src/document.c 2012-11-25 07:16:47.602542000 -0800 @@ -2676,7 +2676,10 @@ void document_undo_add(GeanyDocument *do
g_trash_stack_push(&doc->priv->undo_actions, action);
- document_set_text_changed(doc, TRUE); + if ((type != UNDO_SCINTILLA) || (doc->changed != TRUE)) { + document_set_text_changed(doc, TRUE); + } + ui_update_popup_reundo_items(doc); }
@@ -2840,7 +2843,10 @@ static void document_redo_add(GeanyDocum
g_trash_stack_push(&doc->priv->redo_actions, action);
- document_set_text_changed(doc, TRUE); + if ((type != UNDO_SCINTILLA) || (doc->changed != TRUE)) { + document_set_text_changed(doc, TRUE); + } + ui_update_popup_reundo_items(doc); }
2012/11/25 Dimitar Zhekov dimitar.zhekov@gmail.com
On Sat, 24 Nov 2012 14:09:12 +0000 Nick Treleaven nick.treleaven@btinternet.com wrote:
I played with this a little. It won't work easily because document_set_text_changed is sometimes called to force a UI update, possibly even when the current doc hasn't changed. [...]
On Sat, 24 Nov 2012 15:11:48 -0600 Evandro Borracini evandro.borracini@gmail.com wrote:
Thanks everyone for the comments and suggestions. Based on them, I reverted back to the original code and just added an "if (doc->changed != TRUE)" before calling document_set_text_changed (doc, TRUE) in document_undo/redo_add().
document_redo_add(doc, UNDO_SCINTILLA, NULL); <-- text changed document_redo_add(doc, UNDO_BOM, GINT_TO_POINTER(doc->has_bom)); document_redo_add(doc, UNDO_ENCODING, g_strdup(doc->encoding));
The 2nd and 3rd call probably rely on redo_add to call document_set_text_changed(), to update the status bar. You can check that (my Geany version is a bit outdated), and update the patch to call set_text for BOM/ENCODING (they are rare). Same for undo.
Aside from that, the patch looks reasonable. For plugins, we allow document_set_text_changed(), but not undo/redo_add(), which is OK.
-- E-gards: Jimmy _______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
On Sun, 25 Nov 2012 09:42:41 -0600 Evandro Borracini evandro.borracini@gmail.com wrote:
Thanks Dimitar,
I've updated the patch to always call document_set_text_changed() for BOM/ENCODING. For UNDO_SCINTILLA, document_set_text_changed() is called only at the first document change.
Looks fine to me, though I can't test it ATM. Colomban, Nick?
I've tested and it worked fine for me. The latency while typing is gone. As expected, we still have a small latency at the first key hit but that doesn't create any trouble at all.
On 25/11/2012 15:42, Evandro Borracini wrote:
Thanks Dimitar,
I've updated the patch to always call document_set_text_changed() for BOM/ENCODING. For UNDO_SCINTILLA, document_set_text_changed() is called only at the first document change.
I've tested and it worked fine for me. The latency while typing is gone. As expected, we still have a small latency at the first key hit but that doesn't create any trouble at all.
The new patch is below.
Thanks, applied (with minor edits). I had trouble applying the patch as I had to convert it to tab indentation first. (Also in future please attach patches as a file instead of inline, as this is easier for us and more robust).