Re: [Geany-Devel] Geany performance

2013-09-29 Thread Pavel Roschin
 pos=0 is the default when a new document is opened, only documents in the
 previous session will be opened with pos != 0.  You need to call
 set_cursor_position() to see if any command line options requested a
 position other than zero, or you will break opening new documents from the
 command line.  Note, since the command line options are line and col they
 can't be translated into pos until the document is opened, so they can't
 pass a pos to document_open().

Not sure that my patch broke something but maybe I found more acceptable
solution: add into sci_goto_pos function this:

if(pos == sci_get_current_position(sci))
return;

This has the same effect but it's logically clearly.

--
Best regards,
Pavel Roschin aka RPG
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Geany performance

2013-09-29 Thread Pavel Roschin

 Agreed. But here's the thing: every single file that is loaded needs to be:
 
 1) Loaded from disk (slow)
- Malloc a C string for whole document
 2) Copied into scintilla buffer
- Freed whole document string
 3) Completely lexed by Scintilla lexer (blocking for huge docs)
 4) Grabbing the raw char* of the buffer from Scintilla.
 5) ** Does it take a whole other copy here? **
 6) Passing through CTags and TM (blocking for huge docs).
 7) Updating the user CTags into Scintilla keywords. (SCI_SETKEYWORDS 
 with Ctags results)
 8) Updating the Symbols tree view with new parsed tags.
 9) Focusing the GTK+ notebook page
 10) Setting caret pos to 0 which should be the natural position of the 
 caret but who knows what magic Scintilla does when moving caret.
 
 At present, most of these steps can't be delayed, they all kind of 
 depend on the others due to Geany assuming a single thread on a single 
 core and sharing a bunch of the loaded data globally between modules.

I'm talking about bulk file loading. Loading single file is fast enough, it
slow when you Focusing the GTK+ notebook page for EACH file even if it will
not be ever shown (because there are more files in queue).

 If you have a single project with 10M files, surely you are doing 
 something wrong :)

Google in Chromium project definitely does something wrong?:)

 Indeed we would encourage any contributions here, but just remember it's 
 existing software with certain hardcoded assumptions that have to be 
 lived with. It's easy to bikeshed about how stuff should be, it's a 
 whole different story to actually update the gnarly code to make it a 
 reality and not break everyone's IDE in the process :)

Of course I cannot predict, that simple instruction:

if(pos == sci_get_current_position(sci)) return;

will break something. But if it will, there are _serious_ architecture issues.

But maybe experimenting on beta-testing stage isn't crazy idea?

--
Best regards,
Pavel Roschin aka RPG
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Geany performance

2013-09-29 Thread Lex Trotman
On 29 September 2013 18:31, Pavel Roschin rp...@post.ru wrote:

  pos=0 is the default when a new document is opened, only documents in the
  previous session will be opened with pos != 0.  You need to call
  set_cursor_position() to see if any command line options requested a
  position other than zero, or you will break opening new documents from
 the
  command line.  Note, since the command line options are line and col they
  can't be translated into pos until the document is opened, so they can't
  pass a pos to document_open().

 Not sure that my patch broke something but maybe I found more acceptable
 solution: add into sci_goto_pos function this:

 if(pos == sci_get_current_position(sci))
 return;

 This has the same effect but it's logically clearly.


No, it does not check the command line options, read the code
of set_cursor_position()

Cheers
Lex




 --
 Best regards,
 Pavel Roschin aka RPG

___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Geany performance

2013-09-29 Thread Pavel Roschin
I see. But this function wasn't bottleneck, only sci_goto_pos was.

 On 29 September 2013 18:31, Pavel Roschin rp...@post.ru wrote:
 
   pos=0 is the default when a new document is opened, only documents in the
   previous session will be opened with pos != 0.  You need to call
   set_cursor_position() to see if any command line options requested a
   position other than zero, or you will break opening new documents from
  the
   command line.  Note, since the command line options are line and col they
   can't be translated into pos until the document is opened, so they can't
   pass a pos to document_open().
 
  Not sure that my patch broke something but maybe I found more acceptable
  solution: add into sci_goto_pos function this:
 
  if(pos == sci_get_current_position(sci))
  return;
 
  This has the same effect but it's logically clearly.
 
 
 No, it does not check the command line options, read the code
 of set_cursor_position()
 
 Cheers
 Lex
 
 
 
 
  --
  Best regards,
  Pavel Roschin aka RPG
 




--
Best regards,
Pavel Roschin aka RPG
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Geany performance

2013-09-29 Thread Lex Trotman
On 29 September 2013 19:45, Pavel Roschin rp...@post.ru wrote:

 Shouldn't updateUI called only for _visible_ documents? Anyway it will
 when you
 click on tab and change document.


No it also does notifications, which doesn't depend on visibility.  It
shouldn't render if the document is not visible, but it still should do the
callbacks.  sci_goto_pos() is called in lots of places in Geany, not just
at load time, and probably lots more in plugins, missing notifications has
an unknown impact.




 Considering problem more deeper - actually bottleneck function is
 SCI_ENSUREVISIBLE, not exactly sci_goto_pos. Ok, there are many things to
 thinking about.


Well, the costly part is the rendering, which happens from a delayed GTK
idle callback inside scintilla.  As Matthew said when the text is inserted
into the buffer the cursor is at the end, getting it back to the start
before the rendering is important, the less of the text Scintilla renders
the better, and if the position is set at zero it *should* only render the
start of the text, not the whole file, which would happen if the position
was still at the end.

Cheers
Lex

PS please only reply to the list, not directly to people as well, thats how
stuff gets disconnected from the list.


  On 29 September 2013 18:47, Pavel Roschin rp...@post.ru wrote:
 
It is. If I understand correctly the original reason for this is
 because
when you set the scintilla buffer (at least as it goes currently), it
pushes the cursor to the end of the inserted text, and so it needs
 to be
pushed back to the start by default (if no other position is stored
 or
specified). Please excuse if I'm mistaken here.
  
   Geany with my patch behaves normally without unnecessary goto_pos. But
   today
   I added more clear-looking patch that skips goto-ing if pos ==
 current_pos.
   Isn't? Try, it will take 2 minutes:
  
   void sci_goto_pos(ScintillaObject *sci, gint pos, gboolean unfold)
   {
   if (unfold) SSM(sci, SCI_ENSUREVISIBLE, (uptr_t) SSM(sci,
   SCI_LINEFROMPOSITION, (uptr_t) pos, 0), 0);
   /* Do not goto if we are already here */
   if(pos == sci_get_current_position(sci)) return;
   SSM(sci, SCI_GOTOPOS, (uptr_t) pos, 0);
   }
  
 
  The problem with this is that by not calling the SSM(sci, SCI_GOTOPOS,
  (uptr_t) pos, 0); you don't schedule an updateUI and you then don't get
 the
  associated notifications.  I don't know what impacts that might have,
  especially as plugins can depend on those notifications.
 
  I would be *very* wary changing the semantics of such low level functions
  without great care, checking where they are used and all the side
 effects,
  such as the notification callbacks.
 
  Cheers
  Lex
 
 
  
   --
   Best regards,
   Pavel Roschin aka RPG
   ___
   Devel mailing list
   Devel@lists.geany.org
   https://lists.geany.org/cgi-bin/mailman/listinfo/devel
  




 --
 Best regards,
 Pavel Roschin aka RPG

___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Geany performance

2013-09-29 Thread Lex Trotman
On 29 September 2013 20:53, Pavel Roschin rp...@post.ru wrote:

  Well, the costly part is the rendering, which happens from a delayed GTK
  idle callback inside scintilla.  As Matthew said when the text is
 inserted
  into the buffer the cursor is at the end, getting it back to the start
  before the rendering is important, the less of the text Scintilla renders
  the better, and if the position is set at zero it *should* only render
 the
  start of the text, not the whole file, which would happen if the position
  was still at the end.

 I didn't noticed that somewhere cursor placed at end of file.


When the file is loaded the text is set in the Scintilla buffer, which is
an insert, and the cursor is always after any insert, ie at the end.  Its
not an explicit set position command.



 Geany calls
 set_cursor_position several times. But actually goto isn't bottleneck, only
 SCI_ENSUREVISIBLE slows things down.


Hmmm, bearing in mind we are now talking about the internals of the
scintilla implementation of (one of) its backends.  It sort of makes sense
that an ensure visible operation requires the lexer to be run to find the
fold points so it can check that they are unfolded, and it is conceivable
that checking visibility might require rendering as well (for the purpose
of measuring what text fits on the screen, not actual display). And those
then have to be done immediately, not delayed.  All of which adds to the
the overhead.

Cheers
Lex


 P.S. I hope now all messages are correct? I had a troubles with my mail
 filters.


Sadly no, it still cced me directly, that means the default reply to is to
you, not the list, thats because the list message arrives second and is
hidden since its a copy.



 --
 Best regards,
 Pavel Roschin aka RPG

___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Geany performance

2013-09-29 Thread Pavel Roschin
 Hmmm, bearing in mind we are now talking about the internals of the
 scintilla implementation of (one of) its backends.  It sort of makes sense
 that an ensure visible operation requires the lexer to be run to find the
 fold points so it can check that they are unfolded, and it is conceivable
 that checking visibility might require rendering as well (for the purpose
 of measuring what text fits on the screen, not actual display). And those
 then have to be done immediately, not delayed.  All of which adds to the
 the overhead.  

And again, it's obviously that we don't need to call this function, if position
equals zero or current position, right?

I don't know what is wrong with ML. I have To field - devel@lists.geany.org
and Copy - your mail. Removed you from copy.

--
Best regards,
Pavel Roschin aka RPG
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Geany performance

2013-09-29 Thread Thomas Martitz

Am 29.09.2013 03:26, schrieb Pavel Roschin:

Geany is very fast of course:) But we could make it faster

I use excellent runtime profiler named crxprof. It helped me to find one
bottleneck in geany's starting logic:

main (100.0% | 0.0% self)
  \_ configuration_open_files (55.5% | 0.0% self)
  | \_ document_open_file_full (55.4% | 0.0% self)
  |   \_ editor_goto_pos (29.7% | 0.0% self)
  | \_ sci_goto_pos (29.5% | 0.0% self)
  |   \_ ScintillaGTK::WndProc (29.5% | 0.0% self)
  | \_ Editor::WndProc (29.5% | 0.0% self)
  |   \_ Editor::EnsureLineVisible (29.5% | 0.0% self)
  | \_ Editor::WrapLines (29.5% | 0.0% self)
  |   \_ Editor::WrapOneLine (21.2% | 0.0% self)
  | \_ Editor::LayoutLine (20.3% | 0.4% self)
  |   \_ PositionCache::MeasureWidths (19.0% | 0.3% self)
  | \_ SurfaceImpl::MeasureWidths (18.7% | 0.4% self)
  |   \_ pango_layout_get_iter (16.1% | 1.1% self)
  | \_ pango_itemize_with_base_dir (5.7% | 0.8% self)

As you see, editor_goto_pos takes 1/3 of _loading_ time!

But it shouldn't be called at least if position if 0, so adding simple
optimization will improve loading speed:

GeanyDocument *document_open_file_full(..)
.
if(pos)
{
/* set the cursor position according to pos,
cl_options.goto_line and cl_options.goto_column */ pos =
set_cursor_position(doc-editor, pos); /* now bring the file in front */
editor_goto_pos(doc-editor, pos, FALSE);
}

Not best solution but it works for me:

Loading time before optimization: 8.952s
Loading time after optimization: 5.799s

Much better, isn't?



I'm surprised this costs so much time.

How did you test all this? Did you use a HDD, SDD or ram disk? I would 
expect that the heavy I/O for loading the documents from disk would 
cause most of the delay.





Other proposals about loading speed:
1) Skip unnecessary operations for EACH document because actually user will see
only LAST loaded document. Other operations could be delayed.



I agree that as much work as possible should be delayed to when the doc 
is displayed first or perhaps just to idle cycles. Doing the full blown 
initial work for each and every doc on startup is a huge scalability 
problem especially if the documents in question will not be displayed 
for a while (or in fact never be displayed).


I think doing all the initlal work by using g_idle_add and only make 
sure it's done asap for the finally displayed document (highest 
priority) would be a huge win.


Best regards.
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel


Re: [Geany-Devel] Geany performance

2013-09-29 Thread Pavel Roschin
 I'm surprised this costs so much time.
 
 How did you test all this? Did you use a HDD, SDD or ram disk? I would 
 expect that the heavy I/O for loading the documents from disk would 
 cause most of the delay.

I just measured loading time, because of repeating tests, all data were taken
from RAM cache. Loading documents doesn't produce heavy I/O - average C source
is 30 Kb.

--
Best regards,
Pavel Roschin aka RPG
___
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel