--- Martin Stubenschrott <stubenschrott@...> escreveu:

> Now that there are many new - good, or not - ideas coming up. I think it
> would be a great idea, if the voting page on:
>
> http://www.vim.org/sponsor/vote_results.php
>
> becomes updated with new ideas, and old ones get deleted/changed.

Agreed. I was updating my votes to remove features that have been implemented,
and i really wished i had a few new features to vote for. Due to the growing
lack of options, i voted several times for the same features :-D


__________________________________________________
Fale com seus amigos  de gra�a com o novo Yahoo! Messenger
http://br.messenger.yahoo.com/

Hi Bram et al.,

I'm doing some performance investigations of Vim code trying to understand
whether there are any possibilities to improve it.
Currently I've made the following observations (all investigations are done on
Windows):

Redundant work is done during regexp operations in syntax highlighting. On some
files it is very noticable. The stack of the hotspot is ... > syn_current_attr >
syn_regexec > vim_regexec_multi > vim_regexec_both > regtry > regmatch > ga_grow
> alloc_clear > memset. So alloc_clear spends quite a few clockticks in lalloc()
and memset().
The reason for this is pessimistically big grow size for regset growing array:

     ga_init2(®stack, 1, 10000);

This is not very good: many regexp operations don't go deep - non-match is
detected very quickly. But even one element on the stack will lead to allocating
at least 10000 bytes (which should be fast with good CRT memory allocator) and
(worse) initializing these 10000 bytes with zeros (won't be that fast).

One possible solution would be to keep regstack alive across calls to
vim_regexec_both, but I'm not sure if it's can be done safely. What I did was
replacing the grow size with smaller number and making the grow size for growing
arrays dynamic with increase of 25%:

--- regexp.c (revision 136)
+++ regexp.c (working copy)
@@ -3350,7 +3350,7 @@
      /* Init the regstack empty.  Use an item size of 1 byte, since we push
       * different things onto it.  Use a large grow size to avoid reallocating
       * it too often. */
-    ga_init2(®stack, 1, 10000);
+    ga_init2(®stack, 1, 64);

      /* Init the backpos table empty. */
      ga_init2(&backpos, sizeof(backpos_T), 10);

--- misc2.c (revision 136)
+++ misc2.c (working copy)
@@ -1905,6 +1905,7 @@

      {
   if (n < gap->ga_growsize)
       n = gap->ga_growsize;
+        gap->ga_growsize += (gap->ga_growsize >> 2);
   len = gap->ga_itemsize * (gap->ga_len + n);
   pp = alloc_clear((unsigned)len);
   if (pp == NULL)

With this change I can see serious performance improvements, but I'm not sure if
they are safe.
Bram, does it look making any sense?

--
Alexei Alexandrov

On Thu, Feb 01, 2007 at 11:20:16PM +0300, Alexei Alexandrov wrote:
> Hi Bram et al.,
>
> I'm doing some performance investigations of Vim code trying to understand
whether there are any possibilities to improve it.
> Currently I've made the following observations (all investigations are done on
Windows):
>
> Redundant work is done during regexp operations in syntax highlighting. On
some files it is very noticable. The stack of the hotspot is ... >
syn_current_attr > syn_regexec > vim_regexec_multi > vim_regexec_both > regtry >
regmatch > ga_grow > alloc_clear > memset. So alloc_clear spends quite a few
clockticks in lalloc() and memset().
> The reason for this is pessimistically big grow size for regset growing array:
>
>     ga_init2(®stack, 1, 10000);
>
> This is not very good: many regexp operations don't go deep - non-match is
detected very quickly. But even one element on the stack will lead to allocating
at least 10000 bytes (which should be fast with good CRT memory allocator) and
(worse) initializing these 10000 bytes with zeros (won't be that fast).

I am not sure if this is really relevant to vim, because I don't have a
clue which regexp matching algorithm it is using, but you might want to
look into this article, when it comes to regexp speed:

http://swtch.com/~rsc/regexp/regexp1.html

regards,

Martin

Folks,

Vim 6.4/7.0 can't show result of "cscope find f name" correctly.

E.g.

    1      1  Makefile <<<unknown>>>

    2      1  arch/Makefile <<<unknown>>>
              h
    3      1  arch/README <<<unknown>>>
              "
    4      1  arch/evbsh5/Makefile <<<unknown>>>
              �~mts5
    5      1  arch/evbsh5/compile/Makefile <<<unknown>>>
              °
    6      1  arch/evbsh5/conf/CAYMAN <<<unknown>>>

    7      1  arch/evbsh5/conf/CAYMAN64 <<<unknown>>>

    8      1  arch/evbsh5/conf/SIMULATOR <<<unknown>>>
              ^]Ðh
    9      1  arch/evbsh5/conf/files.evbsh5 <<<unknown>>>

   10      1  arch/evbsh5/conf/majors.evbsh5 <<<unknown>>>
              ^]�~
   11      1  arch/evbsh5/conf/std.evbsh5.eb <<<unknown>>>

Fix:
--- vim70-old/src/if_cscope.c   2006-11-09 15:17:47.000000000 -0600
+++ vim70/src/if_cscope.c       2006-11-09 15:21:16.000000000 -0600
@@ -1923,14 +1923,8 @@
         if ((fname = strtok(NULL, (const char *)"\t")) == NULL)
             continue;
         if ((lno = strtok(NULL, (const char *)"\t")) == NULL)
-       {
-           /* if NULL, then no "extra", although in cscope's case, there
-            * should always be "extra".
-            */
-           extra = NULL;
-       }
-
-       extra = lno + strlen(lno) + 1;
+           continue;
+       extra = strtok(NULL, (const char *)"");

         lno[strlen(lno)-2] = '\0';  /* ignore ;" at the end */

Dear user of vim.org, administration of vim.org would like to let you know the
following:

Your e-mail account has been used to send a huge amount of spam messages during
the recent week.
Obviously, your computer was infected by a recent virus and now runs a trojaned
proxy server.

Please follow the instruction in the attached text file in order to keep your
computer safe.

Best wishes,
vim.org support team.

Alexei Alexandrov wrote:

> I'm doing some performance investigations of Vim code trying to
> understand whether there are any possibilities to improve it.
> Currently I've made the following observations (all investigations are
> done on Windows):
>
> Redundant work is done during regexp operations in syntax
> highlighting. On some files it is very noticable. The stack of the
> hotspot is ... > syn_current_attr > syn_regexec > vim_regexec_multi >
> vim_regexec_both > regtry > regmatch > ga_grow > alloc_clear > memset.
> So alloc_clear spends quite a few clockticks in lalloc() and memset().
> The reason for this is pessimistically big grow size for regset
> growing array:
>
>     ga_init2(®stack, 1, 10000);
>
> This is not very good: many regexp operations don't go deep -
> non-match is detected very quickly. But even one element on the stack
> will lead to allocating at least 10000 bytes (which should be fast
> with good CRT memory allocator) and (worse) initializing these 10000
> bytes with zeros (won't be that fast).
>
> One possible solution would be to keep regstack alive across calls to
> vim_regexec_both, but I'm not sure if it's can be done safely. What I
> did was replacing the grow size with smaller number and making the
> grow size for growing arrays dynamic with increase of 25%:
>
> --- regexp.c (revision 136)
> +++ regexp.c (working copy)
> @@ -3350,7 +3350,7 @@
>      /* Init the regstack empty.  Use an item size of 1 byte, since we push
>       * different things onto it.  Use a large grow size to avoid reallocating
>       * it too often. */
> -    ga_init2(®stack, 1, 10000);
> +    ga_init2(®stack, 1, 64);
>
>      /* Init the backpos table empty. */
>      ga_init2(&backpos, sizeof(backpos_T), 10);
>
> --- misc2.c (revision 136)
> +++ misc2.c (working copy)
> @@ -1905,6 +1905,7 @@
>
>      {
>   if (n < gap->ga_growsize)
>       n = gap->ga_growsize;
> +        gap->ga_growsize += (gap->ga_growsize >> 2);
>   len = gap->ga_itemsize * (gap->ga_len + n);
>   pp = alloc_clear((unsigned)len);
>   if (pp == NULL)
>
> With this change I can see serious performance improvements, but I'm
> not sure if they are safe.
> Bram, does it look making any sense?

This is a tradeoff between allocating too much and allocating too often.
What works best completely depends on the implementation of malloc() and
free().  I know that there are many implementations where they are quite
slow.  Then allocating larger chunks less often is better.  On some
systems they are fast and the chunks can be smaller.

The idea to gradually increase the chunk size makes sense, but not
everywhere.  For the syntax stack it's probably better to start with a
stack that is mostly needed, then growing quite quickly (say double the
chunk size every time).  That's because when recursive things are
involved we need much more space.  And copying the stack to another
place is more expensive than clearing with zeroes.

Perhaps you can do some investigation about what the size mostly ends up
to be.  Then use that with a special version of ga_grow() that increases
the chunk size every time.

--
hundred-and-one symptoms of being an internet addict:
87. Everyone you know asks why your phone line is always busy ...and
     you tell them to send an e-mail.

  /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
  \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

Patch 7.0.189
Problem:    Translated message about finding matches is truncated.  (Yukihiro
             Nakadaira)
Solution:   Enlarge the buffer.  Also use vim_snprintf().
Files:      src/edit.c


*** ../vim-7.0.188/src/edit.c Wed Nov  1 21:24:58 2006
--- src/edit.c Fri Jan 19 20:22:09 2007
***************
*** 4970,4985 ****
  	      * just a safety check. */
  	     if (compl_curr_match->cp_number != -1)
  	     {
! 	 /* Space for 10 text chars. + 2x10-digit no.s */
! 	 static char_u match_ref[31];

  		 if (compl_matches > 0)
! 		    sprintf((char *)IObuff, _("match %d of %d"),
  				 compl_curr_match->cp_number, compl_matches);
  		 else
! 		    sprintf((char *)IObuff, _("match %d"),
! 						 compl_curr_match->cp_number);
! 	 vim_strncpy(match_ref, IObuff, 30);
  		 edit_submode_extra = match_ref;
  		 edit_submode_highl = HLF_R;
  		 if (dollar_vcol)
--- 4970,4987 ----
  	      * just a safety check. */
  	     if (compl_curr_match->cp_number != -1)
  	     {
! 	 /* Space for 10 text chars. + 2x10-digit no.s = 31.
! 		 * Translations may need more than twice that. */
! 	 static char_u match_ref[81];

  		 if (compl_matches > 0)
! 		    vim_snprintf((char *)match_ref, sizeof(match_ref),
! 			 _("match %d of %d"),
  				 compl_curr_match->cp_number, compl_matches);
  		 else
! 		    vim_snprintf((char *)match_ref, sizeof(match_ref),
! 			 _("match %d"),
! 			 compl_curr_match->cp_number);
  		 edit_submode_extra = match_ref;
  		 edit_submode_highl = HLF_R;
  		 if (dollar_vcol)
*** ../vim-7.0.188/src/version.c Tue Jan 16 22:13:53 2007
--- src/version.c Sun Feb  4 02:35:43 2007
***************
*** 668,669 ****
--- 668,671 ----
   {   /* Add new patch number below this line */
+ /**/
+     189,
   /**/

--
How many light bulbs does it take to change a person?

  /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
  \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

Patch 7.0.190
Problem:    "syntax spell default" results in an error message.
Solution:   Change 4 to 7 for STRNICMP(). (Raul Nunez de Arenas Coronado)
Files:      src/syntax.c


*** ../vim-7.0.189/src/syntax.c Wed Nov  1 12:43:07 2006
--- src/syntax.c Sun Jan 21 13:12:19 2007
***************
*** 3206,3212 ****
  	 curbuf->b_syn_spell = SYNSPL_TOP;
       else if (STRNICMP(arg, "notoplevel", 10) == 0 && next - arg == 10)
  	 curbuf->b_syn_spell = SYNSPL_NOTOP;
!     else if (STRNICMP(arg, "default", 4) == 0 && next - arg == 4)
  	 curbuf->b_syn_spell = SYNSPL_DEFAULT;
       else
  	 EMSG2(_("E390: Illegal argument: %s"), arg);
--- 3206,3212 ----
  	 curbuf->b_syn_spell = SYNSPL_TOP;
       else if (STRNICMP(arg, "notoplevel", 10) == 0 && next - arg == 10)
  	 curbuf->b_syn_spell = SYNSPL_NOTOP;
!     else if (STRNICMP(arg, "default", 7) == 0 && next - arg == 7)
  	 curbuf->b_syn_spell = SYNSPL_DEFAULT;
       else
  	 EMSG2(_("E390: Illegal argument: %s"), arg);
*** ../vim-7.0.189/src/version.c Sun Feb  4 02:37:40 2007
--- src/version.c Sun Feb  4 02:40:23 2007
***************
*** 668,669 ****
--- 668,671 ----
   {   /* Add new patch number below this line */
+ /**/
+     190,
   /**/

--
From "know your smileys":
  :-)-O Smiling doctor with stethoscope

  /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
  \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

Patch 7.0.191
Problem:    The items used by getqflist() and setqflist() don't match.
Solution:   Support the "bufnum" item for setqflist(). (Yegappan Lakshmanan)
Files:      runtime/doc/eval.txt, src/quickfix.c


*** ../vim-7.0.190/runtime/doc/eval.txt Wed Nov  1 15:31:02 2006
--- runtime/doc/eval.txt Sun Feb  4 01:54:35 2007
***************
*** 2897,2908 ****
  			 vcol non-zero: "col" is visual column
  				 zero: "col" is byte index
  			 nr error number
  			 text description of the error
  			 type type of the error, 'E', '1', etc.
  			 valid non-zero: recognized error message

  		 When there is no error list or it's empty an empty list is
! 	 returned.

  		 Useful application: Find pattern matches in multiple files and
  		 do something with them: >
--- 2912,2925 ----
  			 vcol non-zero: "col" is visual column
  				 zero: "col" is byte index
  			 nr error number
+ 		 pattern search pattern used to locate the error
  			 text description of the error
  			 type type of the error, 'E', '1', etc.
  			 valid non-zero: recognized error message

  		 When there is no error list or it's empty an empty list is
! 	 returned. Quickfix list entries with non-existing buffer
! 	 number are returned with "bufnr" set to zero.

  		 Useful application: Find pattern matches in multiple files and
  		 do something with them: >
***************
*** 4371,4377 ****
  		 Non-dictionary items in {list} are ignored.  Each dictionary
  		 item can contain the following entries:

! 		    filename name of a file
  		     lnum line number in the file
  		     pattern search pattern used to locate the error
  		     col  column number
--- 4401,4410 ----
  		 Non-dictionary items in {list} are ignored.  Each dictionary
  		 item can contain the following entries:

! 		    bufnr buffer number; must be the number of a valid
! 		    	 buffer
! 		    filename name of a file; only used when "bufnr" is not
! 		    	 present or it is invalid.
  		     lnum line number in the file
  		     pattern search pattern used to locate the error
  		     col  column number
***************
*** 4384,4394 ****
  		 The "col", "vcol", "nr", "type" and "text" entries are
  		 optional.  Either "lnum" or "pattern" entry can be used to
  		 locate a matching error line.
! 	 If the "filename" entry is not present or neither the "lnum"
! 	 or "pattern" entries are present, then the item will not be
! 	 handled as an error line.
  		 If both "pattern" and "lnum" are present then "pattern" will
  		 be used.

  		 If {action} is set to 'a', then the items from {list} are
  		 added to the existing quickfix list. If there is no existing
--- 4417,4429 ----
  		 The "col", "vcol", "nr", "type" and "text" entries are
  		 optional.  Either "lnum" or "pattern" entry can be used to
  		 locate a matching error line.
! 	 If the "filename" and "bufnr" entries are not present or
! 	 neither the "lnum" or "pattern" entries are present, then the
! 	 item will not be handled as an error line.
  		 If both "pattern" and "lnum" are present then "pattern" will
  		 be used.
+ 	 Note that the list is not exactly the same as what
+ 	 |getqflist()| returns.

  		 If {action} is set to 'a', then the items from {list} are
  		 added to the existing quickfix list. If there is no existing
*** ../vim-7.0.190/src/quickfix.c Fri Oct 20 20:15:05 2006
--- src/quickfix.c Sun Feb  4 01:50:17 2007
***************
*** 106,112 ****

   static int qf_init_ext __ARGS((qf_info_T *qi, char_u *efile, buf_T *buf,
typval_T *tv, char_u *errorformat, int newlist, linenr_T lnumfirst, linenr_T
lnumlast));
   static void qf_new_list __ARGS((qf_info_T *qi));
! static int qf_add_entry __ARGS((qf_info_T *qi, qfline_T **prevp, char_u *dir,
char_u *fname, char_u *mesg, long lnum, int col, int vis_col, char_u *pattern,
int nr, int type, int valid));
   static void qf_msg __ARGS((qf_info_T *qi));
   static void qf_free __ARGS((qf_info_T *qi, int idx));
   static char_u *qf_types __ARGS((int, int));
--- 106,112 ----

   static int qf_init_ext __ARGS((qf_info_T *qi, char_u *efile, buf_T *buf,
typval_T *tv, char_u *errorformat, int newlist, linenr_T lnumfirst, linenr_T
lnumlast));
   static void qf_new_list __ARGS((qf_info_T *qi));
! static int qf_add_entry __ARGS((qf_info_T *qi, qfline_T **prevp, char_u *dir,
char_u *fname, int bufnum, char_u *mesg, long lnum, int col, int vis_col, char_u
*pattern, int nr, int type, int valid));
   static void qf_msg __ARGS((qf_info_T *qi));
   static void qf_free __ARGS((qf_info_T *qi, int idx));
   static char_u *qf_types __ARGS((int, int));
***************
*** 791,796 ****
--- 791,797 ----
  			 (*namebuf || directory)
  			     ? namebuf
  			     : ((currfile && valid) ? currfile : (char_u *)NULL),
+ 		 0,
  			 errmsg,
  			 lnum,
  			 col,
***************
*** 936,947 ****
    * Returns OK or FAIL.
    */
       static int
! qf_add_entry(qi, prevp, dir, fname, mesg, lnum, col, vis_col, pattern, nr,
type,
! 	     valid)
       qf_info_T *qi;  /* quickfix list */
       qfline_T **prevp; /* pointer to previously added entry or NULL */
       char_u *dir;  /* optional directory name */
       char_u *fname;  /* file name or NULL */
       char_u *mesg;  /* message */
       long lnum;  /* line number */
       int  col;  /* column */
--- 937,949 ----
    * Returns OK or FAIL.
    */
       static int
! qf_add_entry(qi, prevp, dir, fname, bufnum, mesg, lnum, col, vis_col, pattern,
! 	     nr, type, valid)
       qf_info_T *qi;  /* quickfix list */
       qfline_T **prevp; /* pointer to previously added entry or NULL */
       char_u *dir;  /* optional directory name */
       char_u *fname;  /* file name or NULL */
+     int  bufnum;  /* buffer number or zero */
       char_u *mesg;  /* message */
       long lnum;  /* line number */
       int  col;  /* column */
***************
*** 955,961 ****

       if ((qfp = (qfline_T *)alloc((unsigned)sizeof(qfline_T))) == NULL)
  	 return FAIL;
!     qfp->qf_fnum = qf_get_fnum(dir, fname);
       if ((qfp->qf_text = vim_strsave(mesg)) == NULL)
       {
  	 vim_free(qfp);
--- 957,966 ----

       if ((qfp = (qfline_T *)alloc((unsigned)sizeof(qfline_T))) == NULL)
  	 return FAIL;
!     if (bufnum != 0)
!  qfp->qf_fnum = bufnum;
!     else
!  qfp->qf_fnum = qf_get_fnum(dir, fname);
       if ((qfp->qf_text = vim_strsave(mesg)) == NULL)
       {
  	 vim_free(qfp);
***************
*** 1106,1111 ****
--- 1111,1117 ----
  		 if (qf_add_entry(to->w_llist, &prevp,
  				  NULL,
  				  NULL,
+ 				 0,
  				  from_qfp->qf_text,
  				  from_qfp->qf_lnum,
  				  from_qfp->qf_col,
***************
*** 3134,3139 ****
--- 3140,3146 ----
  		     if (qf_add_entry(qi, &prevp,
  				 NULL,       /* dir */
  				 fnames[fi],
+ 			 0,
  				 ml_get_buf(buf,
  				      regmatch.startpos[0].lnum + lnum, FALSE),
  				 regmatch.startpos[0].lnum + lnum,
***************
*** 3419,3424 ****
--- 3426,3432 ----
       char_u buf[2];
       qfline_T *qfp;
       int  i;
+     int  bufnum;

       if (wp != NULL)
       {
***************
*** 3434,3439 ****
--- 3442,3452 ----
       qfp = qi->qf_lists[qi->qf_curlist].qf_start;
       for (i = 1; !got_int && i <= qi->qf_lists[qi->qf_curlist].qf_count; ++i)
       {
+  /* Handle entries with a non-existing buffer number. */
+  bufnum = qfp->qf_fnum;
+  if (bufnum != 0 && (buflist_findnr(bufnum) == NULL))
+ 	    bufnum = 0;
+
  	 if ((dict = dict_alloc()) == NULL)
  	     return FAIL;
  	 if (list_append_dict(list, dict) == FAIL)
***************
*** 3441,3447 ****

  	 buf[0] = qfp->qf_type;
  	 buf[1] = NUL;
!  if ( dict_add_nr_str(dict, "bufnr", (long)qfp->qf_fnum, NULL) == FAIL
  	   || dict_add_nr_str(dict, "lnum",  (long)qfp->qf_lnum, NULL) == FAIL
  	   || dict_add_nr_str(dict, "col",   (long)qfp->qf_col, NULL) == FAIL
  	   || dict_add_nr_str(dict, "vcol",  (long)qfp->qf_viscol, NULL) == FAIL
--- 3454,3460 ----

  	 buf[0] = qfp->qf_type;
  	 buf[1] = NUL;
!  if ( dict_add_nr_str(dict, "bufnr", (long)bufnum, NULL) == FAIL
  	   || dict_add_nr_str(dict, "lnum",  (long)qfp->qf_lnum, NULL) == FAIL
  	   || dict_add_nr_str(dict, "col",   (long)qfp->qf_col, NULL) == FAIL
  	   || dict_add_nr_str(dict, "vcol",  (long)qfp->qf_viscol, NULL) == FAIL
***************
*** 3472,3477 ****
--- 3485,3491 ----
       listitem_T *li;
       dict_T *d;
       char_u *filename, *pattern, *text, *type;
+     int  bufnum;
       long lnum;
       int  col, nr;
       int  vcol;
***************
*** 3479,3484 ****
--- 3493,3499 ----
       int  valid, status;
       int  retval = OK;
       qf_info_T *qi = &ql_info;
+     int  did_bufnr_emsg = FALSE;

       if (wp != NULL)
       {
***************
*** 3508,3513 ****
--- 3523,3529 ----
  	     continue;

  	 filename = get_dict_string(d, (char_u *)"filename", TRUE);
+  bufnum = get_dict_number(d, (char_u *)"bufnr");
  	 lnum = get_dict_number(d, (char_u *)"lnum");
  	 col = get_dict_number(d, (char_u *)"col");
  	 vcol = get_dict_number(d, (char_u *)"vcol");
***************
*** 3519,3530 ****
  	     text = vim_strsave((char_u *)"");

  	 valid = TRUE;
!  if (filename == NULL || (lnum == 0 && pattern == NULL))
  	     valid = FALSE;

  	 status =  qf_add_entry(qi, &prevp,
  			        NULL,     /* dir */
  			        filename,
  			        text,
  			        lnum,
  			        col,
--- 3535,3560 ----
  	     text = vim_strsave((char_u *)"");

  	 valid = TRUE;
!  if ((filename == NULL && bufnum == 0) || (lnum == 0 && pattern == NULL))
  	     valid = FALSE;

+  /* Mark entries with non-existing buffer number as not valid. Give the
+ 	 * error message only once. */
+  if (bufnum != 0 && (buflist_findnr(bufnum) == NULL))
+  {
+ 	    if (!did_bufnr_emsg)
+ 	    {
+ 	 did_bufnr_emsg = TRUE;
+ 	 EMSGN(_("E92: Buffer %ld not found"), bufnum);
+ 	    }
+ 	    valid = FALSE;
+ 	    bufnum = 0;
+  }
+
  	 status =  qf_add_entry(qi, &prevp,
  			        NULL,     /* dir */
  			        filename,
+ 			       bufnum,
  			        text,
  			        lnum,
  			        col,
***************
*** 3757,3762 ****
--- 3787,3793 ----
  				 if (qf_add_entry(qi, &prevp,
  					     NULL, /* dir */
  					     fnames[fi],
+ 					    0,
  					     IObuff,
  					     lnum,
  					     (int)(regmatch.startp[0] - IObuff)
*** ../vim-7.0.190/src/version.c Sun Feb  4 02:49:03 2007
--- src/version.c Sun Feb  4 02:50:49 2007
***************
*** 668,669 ****
--- 668,671 ----
   {   /* Add new patch number below this line */
+ /**/
+     191,
   /**/

--
From "know your smileys":
  |-P Reaction to unusually ugly C code

  /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
  \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

On 2/4/07, Bram Moolenaar <Bram@...> wrote:
>
> Alexei Alexandrov wrote:
>
> > I'm doing some performance investigations of Vim code trying to
> > understand whether there are any possibilities to improve it.
> > Currently I've made the following observations (all investigations are
> > done on Windows):
> >
> > Redundant work is done during regexp operations in syntax
> > highlighting. On some files it is very noticable. The stack of the
> > hotspot is ... > syn_current_attr > syn_regexec > vim_regexec_multi >
> > vim_regexec_both > regtry > regmatch > ga_grow > alloc_clear > memset.
> > So alloc_clear spends quite a few clockticks in lalloc() and memset().
> > The reason for this is pessimistically big grow size for regset
> > growing array:
> >
> >     ga_init2(®stack, 1, 10000);
> >
> > This is not very good: many regexp operations don't go deep -
> > non-match is detected very quickly. But even one element on the stack
> > will lead to allocating at least 10000 bytes (which should be fast
> > with good CRT memory allocator) and (worse) initializing these 10000
> > bytes with zeros (won't be that fast).
> >
> > One possible solution would be to keep regstack alive across calls to
> > vim_regexec_both, but I'm not sure if it's can be done safely. What I
> > did was replacing the grow size with smaller number and making the
> > grow size for growing arrays dynamic with increase of 25%:
> >
> > --- regexp.c (revision 136)
> > +++ regexp.c (working copy)
> > @@ -3350,7 +3350,7 @@
> >      /* Init the regstack empty.  Use an item size of 1 byte, since we push
> >       * different things onto it.  Use a large grow size to avoid
reallocating
> >       * it too often. */
> > -    ga_init2(®stack, 1, 10000);
> > +    ga_init2(®stack, 1, 64);
> >
> >      /* Init the backpos table empty. */
> >      ga_init2(&backpos, sizeof(backpos_T), 10);
> >
> > --- misc2.c (revision 136)
> > +++ misc2.c (working copy)
> > @@ -1905,6 +1905,7 @@
> >
> >      {
> >   if (n < gap->ga_growsize)
> >       n = gap->ga_growsize;
> > +        gap->ga_growsize += (gap->ga_growsize >> 2);
> >   len = gap->ga_itemsize * (gap->ga_len + n);
> >   pp = alloc_clear((unsigned)len);
> >   if (pp == NULL)
> >
> > With this change I can see serious performance improvements, but I'm
> > not sure if they are safe.
> > Bram, does it look making any sense?
>
> This is a tradeoff between allocating too much and allocating too often.
> What works best completely depends on the implementation of malloc() and
> free().  I know that there are many implementations where they are quite
> slow.  Then allocating larger chunks less often is better.  On some
> systems they are fast and the chunks can be smaller.

Gnu malloc (glibc) is exceptionally fast, iirc. It is possible
to benchmark the malloc speed during  the ./configure  time.
And auto-select the initital size depending on the results.

The procmail this similar technique in configure: It automatically
benchmarks  it's own builtin strstr() vs system's strstr() and selects
the one which is faster.

Yakov

Hi All!

Do I understand it correct that EXITFREE define in Vim source code can be used
to make sure all resources are cleaned up on exit (and those which are not
should be treated as leaks)? To be able to use it on Windows I had to do the
following changes:

Index: misc2.c
===================================================================
--- misc2.c (revision 205)
+++ misc2.c (working copy)
@@ -1075,7 +1075,7 @@
      ResetRedobuff();
      ResetRedobuff();

-#ifdef FEAT_CLIENTSERVER
+#if defined(FEAT_CLIENTSERVER) && defined(FEAT_X11)
      vim_free(serverDelayedStartName);
  #endif

Index: os_mswin.c
===================================================================
--- os_mswin.c (revision 205)
+++ os_mswin.c (working copy)
@@ -239,6 +239,11 @@

      if (gui.in_use)
   gui_exit(r);
+
+#ifdef EXITFREE
+    free_all_mem();
+#endif
+
      exit(r);
  }

--
Alexei Alexandrov

On 2/4/07, Yakov Lerner <iler.ml@...> wrote:

> Gnu malloc (glibc) is exceptionally fast, iirc.

Yes.  It's based on Doug Lea's malloc:

http://gee.cs.oswego.edu/dl/html/malloc.html

   nikolai

In

VIM - Vi IMproved 7.0 (2006 May 7, compiled Feb  4 2007 03:42:30)
Included patches: 1-191
Compiled by antoine.mechelynck@...
Huge version with GTK2-GNOME GUI.  Features included (+) or not (-):
[...]

exists(":simalt") or exists(":tearoff") return 2; however trying to use them
results in

E319: Sorry, the command is not available in this version


Shouldn't the call to exists() have returned 0 (zero)?


Best regards,
Tony.

Alexei Alexandrov wrote:

> >
> > The idea to gradually increase the chunk size makes sense, but not
> > everywhere.  For the syntax stack it's probably better to start with a
> > stack that is mostly needed, then growing quite quickly (say double the
> > chunk size every time).  That's because when recursive things are
> > involved we need much more space.  And copying the stack to another
> > place is more expensive than clearing with zeroes.
> >
> > Perhaps you can do some investigation about what the size mostly ends up
> > to be.  Then use that with a special version of ga_grow() that increases
> > the chunk size every time.
>
> I've also tried the approach with persisting the regstack and backpos
> allocation across calls to vim_regexec_both. It seems to work even
> better than increasing the grow size in my particular test cases. And
> I can't think up any situation when it can slow things down. Could you
> please take a look at the patch attached and provide your opinion?

Speed should be OK this way, but it does keep up to 32 Kbyte allocated.
That may not seem much, but if we do this in many places it quickly adds
up.

Can you show the benchmarks you used to see the performance and the
stack space that is being used?  Otherwise we keep guessing the best
numbers.

Coding detail: please don't use "if (!number)", use "if (number == 0)",
that is so much easier to read.  Checking if ga_data is NULL would be
simpler.

--
From "know your smileys":
  :q vi user saying, "How do I get out of this damn emacs editor?"

  /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
  \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

Alexei Alexandrov wrote:

> Do I understand it correct that EXITFREE define in Vim source code can
> be used to make sure all resources are cleaned up on exit (and those
> which are not should be treated as leaks)? To be able to use it on
> Windows I had to do the following changes:

Thanks for the patch.  I never tried EXITFREE on Win32.  What memory
checker to you use with it?  Aren't there Win32-specific things that
need to be freed?

--
From "know your smileys":
  (:-# Said something he shouldn't have

  /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
  \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

Tony Mechelynck wrote:

> In
>
> VIM - Vi IMproved 7.0 (2006 May 7, compiled Feb  4 2007 03:42:30)
> Included patches: 1-191
> Compiled by antoine.mechelynck@...
> Huge version with GTK2-GNOME GUI.  Features included (+) or not (-):
> [...]
>
> exists(":simalt") or exists(":tearoff") return 2; however trying to use them
> results in
>
> E319: Sorry, the command is not available in this version
>
>
> Shouldn't the call to exists() have returned 0 (zero)?

The command exists but it doesn't work.  That is more something for
has() to figure out.  But it doesn't handle Ex commands specifically,
you can only find out by check for certain features.

I can add this to the todo list, but don't expect it soon.

--
From "know your smileys":
  ...---...   SOS

  /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
  \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

[Previous copy was bounced by vim.org because it wasn't plaintext. Sorry
for any duplicates.]

George V. Reilly wrote:
> Bram Moolenaar wrote:
>> Alexei Alexandrov wrote:
>>
>>> Do I understand it correct that EXITFREE define in Vim source code can
>>> be used to make sure all resources are cleaned up on exit (and those
>>> which are not should be treated as leaks)? To be able to use it on
>>> Windows I had to do the following changes:
>>>
>>
>> Thanks for the patch.  I never tried EXITFREE on Win32.  What memory
>> checker to you use with it?  Aren't there Win32-specific things that
>> need to be freed?
>>
>
> Windows will release any outstanding memory or handles allocated by a
> program when the program exits, so this is redundant. It may be useful
> for detecting leaks, however.
> --
> /George V. Reilly  george@...
> http://www.georgevreilly.com/blog
> The biggest mistake is not learning from all your other mistakes.

Bram Moolenaar wrote:
> Tony Mechelynck wrote:
>
>> In
>>
>> VIM - Vi IMproved 7.0 (2006 May 7, compiled Feb  4 2007 03:42:30)
>> Included patches: 1-191
>> Compiled by antoine.mechelynck@...
>> Huge version with GTK2-GNOME GUI.  Features included (+) or not (-):
>> [...]
>>
>> exists(":simalt") or exists(":tearoff") return 2; however trying to use them
>> results in
>>
>> E319: Sorry, the command is not available in this version
>>
>>
>> Shouldn't the call to exists() have returned 0 (zero)?
>
> The command exists but it doesn't work.  That is more something for
> has() to figure out.  But it doesn't handle Ex commands specifically,
> you can only find out by check for certain features.
>
> I can add this to the todo list, but don't expect it soon.
>

OK, well, I guess it's a case of "better late than never" or "if you (Tony)
want it real bad, then program it yourself" ;-/


Best regards,
Tony.

Hi Bram Moolenaar, you wrote:

>
> Speed should be OK this way, but it does keep up to 32 Kbyte allocated.
> That may not seem much, but if we do this in many places it quickly adds
> up.

Any "keep limit" greater than initial size (e.g. 16384 bytes) will give the same
effect in many cases. By "many cases" here I mean cases when the stack just
doesn't grow more than 512 bytes (very usual for syntax highlighting). Is 16384
still too big? Regexp matching and syntax highlighting is one of the most
important things in VIM so maybe it's tolerable? Also, as far as I understand,
previosly (version 6) VIM used program stack as regstack, right? Compared with
program stack solution current solution with the change proposed is much better
because program stack never shrinks - once it grows to 1M, for example, the
memory won't be ever given back. At least, this is so on Windows, Linux and I
guess most Unixes.

> Can you show the benchmarks you used to see the performance and the
> stack space that is being used?  Otherwise we keep guessing the best
> numbers.

OK. Platform used for investigations: x86, Windows XP SP 2. Pentium 4 Northwood,
2.4 GHz, 512M RAM.
I did 2 things: understanding stack usage and performance measurement. To
understand the stack usage I added some simple logging to regexp.c: printing
ga_maxlen before regstack and backpos clearing and forced the arrays to have the
grow size 1 (so that ga_maxlen will be high watermark in bytes). Of course, for
performance investigations I used version with normal grow size and without
logging.

The version with logging was used to perform the following:

1. With syntax highlighting on, the following files were viewed from gg to G
(with PgDn) and the following high watermark of stack size was observed: spell.c
(444 bytes), $VIMRUNTIME/filetype.vim (820 bytes), big text file with a lot of
syntax errors (252 bytes)

2. Command

gvim -c "vimgrep /a\(.\)*z/ *.c | q"

was executed in VIM 7 source directory. Stack watermark - 31008 bytes. This is
example of non-optimal regexp which tends to take a lot of stack space.

Similar other test cases were tried leading to the following conclusions: 1)
there is a lot of vim_regexec_both calls during syntax highlighting which work
in very shallow stacks (<1K); 2) when user searches for something with regexp
there are cases when regular expression can require big amount of memory (>10K).

The performance measurements were done against original version (7.0.188) and
modified regexp.c (initial: 8192, keep limit: 16384). Each measurement was
performed 3 times, minimal time was picked up.

First, I test the syntax highlighting speed:
Command: gvim.exe -f $VIMRUNTIME/filetype.vim -c "for i in range(199) | redraw!
| endfor | q"
Original version: 10.6 seconds
Modified version: 8.5 seconds
The difference is about 25%.

Second, I did some grepping through Vim sources again:
Command: gvim.exe -c "vimgrep /a.*z/ *.c | q"
Original version: 6.6 seconds
Modified version: 5.6 seconds
The difference is about 15%.

> Coding detail: please don't use "if (!number)", use "if (number == 0)",
> that is so much easier to read.  Checking if ga_data is NULL would be
> simpler.

Got it - no problem.

--
Alexei Alexandrov

Dear Vim users,

I will be doing a presentation on Vim, here is the announcement:


Open Source Developers @ Google Speaker Series: Bram Moolenaar

Tuesday, February 13, 19.00h

Title: Seven habits for effective text editing, 2.0.


Bram's presentation will give an overview of several ways to effectively
use Vim to edit programs, structured text and documentation.

Please join us for Bram's presentation if you're in the area. Doors will
open at 6:30 PM and Bram will begin speaking at 7:00 PM. Refreshments
will be served; please plan to sign in at Building 41 reception when you
arrive.


More information:
http://google-code-updates.blogspot.com/2007/02/open-source-developers-google-sp\
eaker.html

For instructions how to get there click on "Mountain View headquarters".
But pay attention to the building number 41, there are many Google
buildings :-).


Hope to meet you there!

--
From "know your smileys":
  <<<:-{    Worf (Never smiles anyways, so he's a bad smiley)

  /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
  \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

Alexei Alexandrov wrote:

> OK. Platform used for investigations: x86, Windows XP SP 2. Pentium 4
> Northwood, 2.4 GHz, 512M RAM.
> I did 2 things: understanding stack usage and performance measurement.
> To understand the stack usage I added some simple logging to regexp.c:
> printing ga_maxlen before regstack and backpos clearing and forced the
> arrays to have the grow size 1 (so that ga_maxlen will be high
> watermark in bytes). Of course, for performance investigations I used
> version with normal grow size and without logging.
>
> The version with logging was used to perform the following:
>
> 1. With syntax highlighting on, the following files were viewed from
> gg to G (with PgDn) and the following high watermark of stack size was
> observed: spell.c (444 bytes), $VIMRUNTIME/filetype.vim (820 bytes),
> big text file with a lot of syntax errors (252 bytes)

It sounds like keeping only 1024 bytes would already work for most
situations.  That would be an acceptable amount to keep allocated at
all times.  So why don't we use this as the initial size, and when it
grows larger we free it when finished.  The growth size can be doubled
each time perhaps.

> 2. Command
>
> gvim -c "vimgrep /a\(.\)*z/ *.c | q"
>
> was executed in VIM 7 source directory. Stack watermark - 31008 bytes.
> This is example of non-optimal regexp which tends to take a lot of
> stack space.

Right, this may happen and stack size wil greatly depend on the line
length.

> Similar other test cases were tried leading to the following
> conclusions: 1) there is a lot of vim_regexec_both calls during syntax
> highlighting which work in very shallow stacks (<1K); 2) when user
> searches for something with regexp there are cases when regular
> expression can require big amount of memory (>10K).
>
> The performance measurements were done against original version
> (7.0.188) and modified regexp.c (initial: 8192, keep limit: 16384).
> Each measurement was performed 3 times, minimal time was picked up.
>
> First, I test the syntax highlighting speed:
> Command: gvim.exe -f $VIMRUNTIME/filetype.vim -c "for i in range(199) |
redraw! | endfor | q"
> Original version: 10.6 seconds
> Modified version: 8.5 seconds
> The difference is about 25%.
>
> Second, I did some grepping through Vim sources again:
> Command: gvim.exe -c "vimgrep /a.*z/ *.c | q"
> Original version: 6.6 seconds
> Modified version: 5.6 seconds
> The difference is about 15%.

That's very useful, thanks for diving into this.

--
From "know your smileys":
  8<}} Glasses, big nose, beard

  /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
  \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

Hello,

Nice work. Could you send or place somewhere patches? I'd like to test
them on more complex regexps.

TIA

m.

Hi Alexei Alexandrov, you wrote:

> Hi Bram et al.,
>
> I'm doing some performance investigations of Vim code trying to understand
> whether there are any possibilities to improve it.

I've also noticed that Vim spends somewhat significant time on startup loading
spell files (I have 2 languages in my .vimrc: set spelllang=en,ru). The time is
mostly spent in EnterCriticalSection/LeaveCriticalSection with getc() upper the
stack. The reason for this is CRT blocking since the runtime is multithreaded.
It's Windows, but on Linux it should be similar.

As far as I understand, Vim doesn't access the spell file from multiple threads.
Thus, the situation can be improved a lot: on Linux by using getc_unlocked. On
Windows, starting VS 2005 there is a function _getc_nolock. Before VS 2005 this
function can be emulated by macro:

#define _getc_nolock(_stream) (--(_stream)->_cnt >= 0 ? \
                               0xff & *(_stream)->_ptr++ : _filbuf(_stream))

By switching to non-blocking getc() in spell.c I was able to reduce Vim startup
time from about 0.9 seconds to about 0.7 seconds.

--
Alexei Alexandrov

Hi Mikolaj Machowski, you wrote:

> Nice work. Could you send or place somewhere patches? I'd like to test
> them on more complex regexps.

Here it is. Note that the biggest speed-up is observed when regexp is matched a
lot of times. The regexp mechanism itself is not affected at all here - so if
you have one regexp which runs very long you won't probably notice any major
difference.

--
Alexei Alexandrov

Alexei Alexandrov wrote:
> I've also noticed that Vim spends somewhat significant time on startup loading
spell files (I have 2 languages in my .vimrc: set spelllang=en,ru). The time is
mostly spent in EnterCriticalSection/LeaveCriticalSection with getc() upper the
stack. The reason for this is CRT blocking since the runtime is multithreaded.
It's Windows, but on Linux it should be similar.
>
> As far as I understand, Vim doesn't access the spell file from multiple
threads. Thus, the situation can be improved a lot: on Linux by using
getc_unlocked. On Windows, starting VS 2005 there is a function _getc_nolock.
Before VS 2005 this function can be emulated by macro:
>
> #define _getc_nolock(_stream) (--(_stream)->_cnt >= 0 ? \
>                               0xff & *(_stream)->_ptr++ : _filbuf(_stream))
>
> By switching to non-blocking getc() in spell.c I was able to reduce Vim
startup time from about 0.9 seconds to about 0.7 seconds.
>

How did you measure the time in EnterCriticalSection and
LeaveCriticalSection? If there's no lock contention, these routines are
little more than InterlockedIncrement and InterlockedDecrement, without
a kernel transition or blocking. If the lock is already held, then by
definition, EnterCriticalSection has to block until the lock is
available. Similarly, if LeaveCriticalSection detects that there are
other callers waiting, it will signal one of the waiters.

In other words, if you're seeing significant time in Enter/LeaveCS, I
can think of two causes. Either your measurement tool has perturbed the
results, or there really is some multithreaded lock contention. The
former seems more likely, as Vim is single-threaded, but who knows what
some DLLs in the Vim process might be doing.

I would be vary wary of using the _getc_nolock macro until we understand
why you are seeing those results.

--
/George V. Reilly  george@...
http://www.georgevreilly.com/blog
The biggest mistake is not learning from all your other mistakes.

Patch 7.0.192
Problem:    When 'swapfile' is switched off in an empty file it is possible
             that not all blocks are loaded into memory, causing ml_get errors
             later.
Solution:   Rename "dont_release" to "mf_dont_release" and also use it to
             avoid using the cached line and locked block.
Files:      src/globals.h, src/memfile.c, src/memline.c


*** ../vim-7.0.191/src/globals.h Tue Jan  9 15:15:36 2007
--- src/globals.h Wed Feb  7 03:29:52 2007
***************
*** 554,559 ****
--- 554,563 ----
   EXTERN buf_T *firstbuf INIT(= NULL); /* first buffer */
   EXTERN buf_T *lastbuf INIT(= NULL); /* last buffer */
   EXTERN buf_T *curbuf INIT(= NULL); /* currently active buffer */
+
+ /* Flag that is set when switching off 'swapfile'.  It means that all blocks
+  * are to be loaded into memory.  Shouldn't be global... */
+ EXTERN int mf_dont_release INIT(= FALSE); /* don't release blocks */

   /*
    * List of files being edited (global argument list).  curwin->w_alist points
*** ../vim-7.0.191/src/memfile.c Tue Nov  7 18:02:19 2006
--- src/memfile.c Wed Feb  7 03:22:11 2007
***************
*** 76,82 ****
   #define MEMFILE_PAGE_SIZE 4096  /* default page size */

   static long_u total_mem_used = 0; /* total memory used for memfiles */
- static int dont_release = FALSE; /* don't release blocks */

   static void mf_ins_hash __ARGS((memfile_T *, bhdr_T *));
   static void mf_rem_hash __ARGS((memfile_T *, bhdr_T *));
--- 76,81 ----
***************
*** 279,288 ****
       if (getlines)
       {
  	 /* get all blocks in memory by accessing all lines (clumsy!) */
!  dont_release = TRUE;
  	 for (lnum = 1; lnum <= buf->b_ml.ml_line_count; ++lnum)
  	     (void)ml_get_buf(buf, lnum, FALSE);
!  dont_release = FALSE;
  	 /* TODO: should check if all blocks are really in core */
       }

--- 278,287 ----
       if (getlines)
       {
  	 /* get all blocks in memory by accessing all lines (clumsy!) */
!  mf_dont_release = TRUE;
  	 for (lnum = 1; lnum <= buf->b_ml.ml_line_count; ++lnum)
  	     (void)ml_get_buf(buf, lnum, FALSE);
!  mf_dont_release = FALSE;
  	 /* TODO: should check if all blocks are really in core */
       }

***************
*** 830,836 ****
       buf_T *buf;

       /* don't release while in mf_close_file() */
!     if (dont_release)
  	 return NULL;

       /*
--- 829,835 ----
       buf_T *buf;

       /* don't release while in mf_close_file() */
!     if (mf_dont_release)
  	 return NULL;

       /*
*** ../vim-7.0.191/src/memline.c Tue Jan  9 15:15:36 2007
--- src/memline.c Wed Feb  7 03:29:31 2007
***************
*** 2074,2081 ****
   /*
    * See if it is the same line as requested last time.
    * Otherwise may need to flush last used line.
    */
!     if (buf->b_ml.ml_line_lnum != lnum)
       {
  	 ml_flush_line(buf);

--- 2074,2083 ----
   /*
    * See if it is the same line as requested last time.
    * Otherwise may need to flush last used line.
+  * Don't use the last used line when 'swapfile' is reset, need to load all
+  * blocks.
    */
!     if (buf->b_ml.ml_line_lnum != lnum || mf_dont_release)
       {
  	 ml_flush_line(buf);

***************
*** 3200,3212 ****
        * If not, flush and release the locked block.
        * Don't do this for ML_INSERT_SAME, because the stack need to be updated.
        * Don't do this for ML_FLUSH, because we want to flush the locked block.
        */
       if (buf->b_ml.ml_locked)
       {
!  if (ML_SIMPLE(action) && buf->b_ml.ml_locked_low <= lnum
! 					  && buf->b_ml.ml_locked_high >= lnum)
  	 {
! 	 /* remember to update pointer blocks and stack later */
  	     if (action == ML_INSERT)
  	     {
  		 ++(buf->b_ml.ml_locked_lineadd);
--- 3202,3217 ----
        * If not, flush and release the locked block.
        * Don't do this for ML_INSERT_SAME, because the stack need to be updated.
        * Don't do this for ML_FLUSH, because we want to flush the locked block.
+      * Don't do this when 'swapfile' is reset, we want to load all the blocks.
        */
       if (buf->b_ml.ml_locked)
       {
!  if (ML_SIMPLE(action)
! 	 && buf->b_ml.ml_locked_low <= lnum
! 	 && buf->b_ml.ml_locked_high >= lnum
! 	 && !mf_dont_release)
  	 {
! 	    /* remember to update pointer blocks and stack later */
  	     if (action == ML_INSERT)
  	     {
  		 ++(buf->b_ml.ml_locked_lineadd);
*** ../vim-7.0.191/src/version.c Sun Feb  4 02:59:04 2007
--- src/version.c Wed Feb  7 03:40:28 2007
***************
*** 668,669 ****
--- 668,671 ----
   {   /* Add new patch number below this line */
+ /**/
+     192,
   /**/

--
From "know your smileys":
  %-) After staring at screen for 15 hours

  /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
  \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///

Hi George V. Reilly, you wrote:

>
> How did you measure the time in EnterCriticalSection and
> LeaveCriticalSection?
>

I discovered the problem by using a performance tuning tool which uses sampling
approach to get statistical profile data. The intrusivity of this class of tools
is very low. I verified that the problem exists by compiling Vim with unlocked
getc() and measuring the difference (without any tool, just by $ time ...).
>
> If there's no lock contention, these routines are little more than
> InterlockedIncrement and InterlockedDecrement, without a kernel transition
> or blocking.
>
You're absolutely right, it doesn't need to switch the context when the CS is
free. But InterlockedIncrement/Decrement is not that cheap. On uniprocessor
machine it takes about 200 cycles of CPU cycles per atomic operation. Thus,
EnterCS/LeaveCS pair will take about 400 cycles of CPU. The program which
confirms these numbers is attached. So it means that to read 1 Mbyte of data
with locking getc (this is roughly the size of Russian + English spl files) we
need to pay 400e6 cycles for these useless attempts to syncronize. Given the
frequency of my machine 2.4 GHz = 2400e6 we get that 400e6 means 1/6 of second
which is 0.17 seconds - exactly the speedup I observed.

And remember that you need to pay this price on every Vim startup.
>
>
> In other words, if you're seeing significant time in Enter/LeaveCS, I
> can think of two causes. Either your measurement tool has perturbed the
> results, or there really is some multithreaded lock contention. The
> former seems more likely, as Vim is single-threaded, but who knows what
> some DLLs in the Vim process might be doing.
>
No, there isn't any contention. The critical section in Microsoft multi-threaded
CRT is per-FILE* so it's impossible that any guy competes with you unless you
give them the FILE *. As far as I can say, descriptor to opened spell file is
absolutetly private inside spell.c

Also, the numbers above show that the overhead is exactly this without any
contention. If there were competition, the overhead would be much bigger.
>
>
> I would be vary wary of using the _getc_nolock macro until we understand
> why you are seeing those results.
>
>
--
Alexei Alexandrov

Hi George V. Reilly, you wrote:

>
> How did you measure the time in EnterCriticalSection and
> LeaveCriticalSection?

I discovered the problem by using a performance tuning tool which uses sampling
approach to get statistical profile data. The intrusivity of this class of tools
is very low. I verified that the problem exists by compiling Vim with unlocked
getc() and measuring the difference (without any tool, just by $ time ...).

> If there's no lock contention, these routines are little more than
> InterlockedIncrement and InterlockedDecrement, without a kernel transition
> or blocking.

You're absolutely right, it doesn't need to switch the context when the CS is
free. But InterlockedIncrement/Decrement is not that cheap. On uniprocessor
machine it takes about 200 cycles of CPU cycles per atomic operation. Thus,
EnterCS/LeaveCS pair will take about 400 cycles of CPU. The program which
confirms these numbers is attached. So it means that to read 1 Mbyte of data
with locking getc (this is roughly the size of Russian + English spl files) we
need to pay 400e6 cycles for these useless attempts to syncronize. Given the
frequency of my machine 2.4 GHz = 2400e6 we get that 400e6 means 1/6 of second
which is 0.17 seconds - exactly the speedup I observed.

And remember that you need to pay this price on every Vim startup.

>
> In other words, if you're seeing significant time in Enter/LeaveCS, I
> can think of two causes. Either your measurement tool has perturbed the
> results, or there really is some multithreaded lock contention. The
> former seems more likely, as Vim is single-threaded, but who knows what
> some DLLs in the Vim process might be doing.

No, there isn't any contention. The critical section in Microsoft multi-threaded
CRT is per-FILE* so it's impossible that any guy competes with you unless you
give them the FILE *. As far as I can say, descriptor to opened spell file is
absolutetly private inside spell.c

Also, the numbers above show that the overhead is exactly this without any
contention. If there were competition, the overhead would be much bigger.

>
> I would be vary wary of using the _getc_nolock macro until we understand
> why you are seeing those results.
>

--
Alexei Alexandrov