Loading ...
Sorry, an error occurred while loading the content.

Re: Progress indicator for :TOhtml command

Expand Messages
  • Ben Fritz
    ... I m sorry I don t follow. You re saying that a 4% increase in time for the progress bar, and a 20% decrease without the progress bar, is too slow ? And
    Message 1 of 28 , Jun 5, 2010
    • 0 Attachment
      On Jun 5, 6:26 pm, ZyX <zyx....@...> wrote:
      > A small benchmark for your and mine scripts:
      >                               user    system  cpu  total      Relative
      > mine, no progress              93,07  10,82   99%  1:44,06    + 5%
      > mine, only per cents and bar   95,77  10,92   99%  1:46,94    + 8%
      > mine, %, bar and lines        125,59  14,80   99%  2:20,83    +43%
      > 2html from vim-7.2.303         97,34   1,16   99%  1:38,64    + 0%
      > your 2html, no progress        77,31   0,99   99%  1:18,55    -20%
      > your 2html, with progress     100,57   1,20   99%  1:42,76    + 4%
      >
      > [Snip]
      >
      > I do not know what exactly is a problem, but your progress is too slow.
      >

      I'm sorry I don't follow. You're saying that a 4% increase in time for
      the progress bar, and a 20% decrease without the progress bar, is "too
      slow"?

      And you're proposing changes that make it an 8% increase with the
      progress bar, or a 5% increase without?

      --
      You received this message from the "vim_dev" maillist.
      Do not top-post! Type your reply below the text you are replying to.
      For more information, visit http://www.vim.org/maillist.php
    • Benjamin Fritz
      ... Yes, I did not expect any performance gains from removing the little bit of remaining floating point, since it is just up to 100 calculations done once at
      Message 2 of 28 , Jun 5, 2010
      • 0 Attachment
        On Jun 5, 8:10 pm, ZyX <zyx....@...> wrote:
        >
        > It occures that the problem is not floating-point math: the attached patch
        > removes this math but does not add any perfomance.
        >

        Yes, I did not expect any performance gains from removing the little
        bit of remaining floating point, since it is just up to 100
        calculations done once at the start and thereafter only when the
        window changes size. It is a good idea to remove, because as you point
        out, that amount of precision is probably unnecessary, and it would
        just introduce another dependency.

        > It also removes recalculating
        > progress bar width (you just used used some generic progress bar?) and
        > needs_redraw.

        Yes, we did use a generic progress bar as the starting point for this.
        However, I think it IS necessary to recalculate the progress bar
        width. This is done so that if the user changes window sizes, the
        progress bar will be updated accordingly. We don't want a progress bar
        that is too big to fit in the window, or smaller than needed for
        decent viewing. With your patch, if you start with the gvim window
        maximized, then restore the window to a smaller size, Vim goes blank
        until the next progress bar update, and then the progress bar is too
        large to fit on the screen and is truncated. This is not desirable,
        but perhaps it would acceptable if the performance gains are great
        enough. This does not seem to be the case, because I added back in the
        size recalculation with no noticeable performance hit.

        The needs_redraw was done in order to allow us to call redrawstatus on
        the correct window. :help redrawstatus says that it redraws the status
        line for the *current window* only unless you use redrawstatus! which
        redraws all windows. In practice, however, it does not seem to matter
        which window we use it in. Why is this?

        > Also, why you forbid profiling progress bar functions? It is also
        > fixed.
        >

        Good catch, that's certainly something to include going forward.

        There is a slight speed gain from your patch, however there is a
        mistaken assumption in the way you update the progress bar. Your code
        assumes that the progress bar will only ever update by one tick at a
        time. Updating the progress bar without your patch calculates the
        entire string every time, using repeat(). Your update simply adds one
        to the colored string of spaces, and subtracts one from the uncolored.
        This does not work if the user folds away some text and does not use
        dynamic folding, it does not work when there are fewer than 100 lines
        in the text to convert, and it does not work for the second use of the
        progress bar, where there are usually fewer that 100 highlight groups
        to process.

        I corrected this problem and initially, the performance still seemed
        to be improved over the previous version. However, I noticed afterward
        that part of the patch removes the "sleep 100m" from the "processing
        classes" step. I took this line out of the original script for a fair
        comparison, and got the following timings, converting
        autoload/netrw.vim (7764 lines) with dynamic folding enabled:

        Before patch: 50 seconds
        Patch from ZyX: 49 seconds
        Fixed patch: 51 seconds

        So, it looks like the patch is actually no faster, and potentially
        slightly slower than the precalculated version.

        I have therefore attached an updated version of my last submission,
        which removes floating point from the calculate_ticks function, and
        incorporates some of the other improvements from ZyX.

        This version takes 50 seconds to convert netrw, if I comment out the
        sleep 100 line. Do we want this line in the code? Without it, if there
        are not very many highlight groups to process, the "processing
        classes" bar flashes by without being seen. This happens anyway for
        very small selections. I don't know how I feel about deliberately
        slowing down the execution. I have left it commented out for now.

        I am very curious about this:

        " Note that you must use len(split) instead of len() if you want to use
        " unicode in title
        let self.pb_len = max_len-len(split(self.title, '\zs'))-3-4-2

        Can someone explain the problem described in the comment a little
        better? And why does the split on '\zs' work to fix the problem?

        --
        You received this message from the "vim_dev" maillist.
        Do not top-post! Type your reply below the text you are replying to.
        For more information, visit http://www.vim.org/maillist.php
      • ZyX
        Ответ на сообщение «Re: Progress indicator for :TOhtml command», присланное в 10:59:42 06 июня 2010, Воскресенье,
        Message 3 of 28 , Jun 6, 2010
        • 0 Attachment
          Ответ на сообщение «Re: Progress indicator for :TOhtml command»,
          присланное в 10:59:42 06 июня 2010, Воскресенье,
          отправитель Benjamin Fritz:

          The reason why I say that progress bar is too slow is that my script does not
          suffer from performance decrease unless you make it redraw on each line. I will
          add size recalculation for my script too (I removed it from your script because
          I did not realize that while user can do nothing in vim he still can resize the
          terminal), but I do not think that this will add any performance penalty.

          > I am very curious about this:
          >
          > " Note that you must use len(split) instead of len() if you want to use
          > " unicode in title
          > let self.pb_len = max_len-len(split(self.title, '\zs'))-3-4-2
          >
          > Can someone explain the problem described in the comment a little
          > better? And why does the split on '\zs' work to fix the problem?
          That is because len(str) measures byte length of C string, while len(split) first
          splits the string into a list of characters and then measures the length of
          character list. If there are non-latin1 Unicode symbols and encoding is a
          multibyte one then length of character list is not equal to bytes count of C
          string.

          Текст сообщения:
          > On Jun 5, 8:10 pm, ZyX <zyx....@...> wrote:
          > > It occures that the problem is not floating-point math: the attached
          > > patch removes this math but does not add any perfomance.
          >
          > Yes, I did not expect any performance gains from removing the little
          > bit of remaining floating point, since it is just up to 100
          > calculations done once at the start and thereafter only when the
          > window changes size. It is a good idea to remove, because as you point
          > out, that amount of precision is probably unnecessary, and it would
          > just introduce another dependency.
          >
          > > It also removes recalculating
          > > progress bar width (you just used used some generic progress bar?) and
          > > needs_redraw.
          >
          > Yes, we did use a generic progress bar as the starting point for this.
          > However, I think it IS necessary to recalculate the progress bar
          > width. This is done so that if the user changes window sizes, the
          > progress bar will be updated accordingly. We don't want a progress bar
          > that is too big to fit in the window, or smaller than needed for
          > decent viewing. With your patch, if you start with the gvim window
          > maximized, then restore the window to a smaller size, Vim goes blank
          > until the next progress bar update, and then the progress bar is too
          > large to fit on the screen and is truncated. This is not desirable,
          > but perhaps it would acceptable if the performance gains are great
          > enough. This does not seem to be the case, because I added back in the
          > size recalculation with no noticeable performance hit.
          >
          > The needs_redraw was done in order to allow us to call redrawstatus on
          > the correct window. :help redrawstatus says that it redraws the status
          > line for the *current window* only unless you use redrawstatus! which
          > redraws all windows. In practice, however, it does not seem to matter
          > which window we use it in. Why is this?
          >
          > > Also, why you forbid profiling progress bar functions? It is also
          > > fixed.
          >
          > Good catch, that's certainly something to include going forward.
          >
          > There is a slight speed gain from your patch, however there is a
          > mistaken assumption in the way you update the progress bar. Your code
          > assumes that the progress bar will only ever update by one tick at a
          > time. Updating the progress bar without your patch calculates the
          > entire string every time, using repeat(). Your update simply adds one
          > to the colored string of spaces, and subtracts one from the uncolored.
          > This does not work if the user folds away some text and does not use
          > dynamic folding, it does not work when there are fewer than 100 lines
          > in the text to convert, and it does not work for the second use of the
          > progress bar, where there are usually fewer that 100 highlight groups
          > to process.
          >
          > I corrected this problem and initially, the performance still seemed
          > to be improved over the previous version. However, I noticed afterward
          > that part of the patch removes the "sleep 100m" from the "processing
          > classes" step. I took this line out of the original script for a fair
          > comparison, and got the following timings, converting
          > autoload/netrw.vim (7764 lines) with dynamic folding enabled:
          >
          > Before patch: 50 seconds
          > Patch from ZyX: 49 seconds
          > Fixed patch: 51 seconds
          >
          > So, it looks like the patch is actually no faster, and potentially
          > slightly slower than the precalculated version.
          >
          > I have therefore attached an updated version of my last submission,
          > which removes floating point from the calculate_ticks function, and
          > incorporates some of the other improvements from ZyX.
          >
          > This version takes 50 seconds to convert netrw, if I comment out the
          > sleep 100 line. Do we want this line in the code? Without it, if there
          > are not very many highlight groups to process, the "processing
          > classes" bar flashes by without being seen. This happens anyway for
          > very small selections. I don't know how I feel about deliberately
          > slowing down the execution. I have left it commented out for now.
          >
          > I am very curious about this:
          >
          > " Note that you must use len(split) instead of len() if you want to use
          > " unicode in title
          > let self.pb_len = max_len-len(split(self.title, '\zs'))-3-4-2
          >
          > Can someone explain the problem described in the comment a little
          > better? And why does the split on '\zs' work to fix the problem?
          >
        • ZyX
          Ответ на сообщение «Re: Progress indicator for :TOhtml command», присланное в 10:59:42 06 июня 2010, Воскресенье,
          Message 4 of 28 , Jun 6, 2010
          • 0 Attachment
            Ответ на сообщение «Re: Progress indicator for :TOhtml command»,
            присланное в 10:59:42 06 июня 2010, Воскресенье,
            отправитель Benjamin Fritz:

            It is odd: the only problem in your script is redrawstatus which is called only
            100 times (without styles, 109 with) (>21 seconds), while in my script
            redrawstatus called 328 times takes less than a second.

            Second problem with the whole 2html is buffer switching, I think you should
            consider instead of doing constant switches, save every line in a List and only
            after everything is finished create a new buffer and call setline(1, s:list).
            Note that new versions of my script are faster (but not much) then your 2html
            because I use this technique.

            And, why do you calculate length of the title at each progressbarupdate?
            Attached patch fixes this and the case when there is no space for progress bar.

            Текст сообщения:
            > On Jun 5, 8:10 pm, ZyX <zyx....@...> wrote:
            > > It occures that the problem is not floating-point math: the attached
            > > patch removes this math but does not add any perfomance.
            >
            > Yes, I did not expect any performance gains from removing the little
            > bit of remaining floating point, since it is just up to 100
            > calculations done once at the start and thereafter only when the
            > window changes size. It is a good idea to remove, because as you point
            > out, that amount of precision is probably unnecessary, and it would
            > just introduce another dependency.
            >
            > > It also removes recalculating
            > > progress bar width (you just used used some generic progress bar?) and
            > > needs_redraw.
            >
            > Yes, we did use a generic progress bar as the starting point for this.
            > However, I think it IS necessary to recalculate the progress bar
            > width. This is done so that if the user changes window sizes, the
            > progress bar will be updated accordingly. We don't want a progress bar
            > that is too big to fit in the window, or smaller than needed for
            > decent viewing. With your patch, if you start with the gvim window
            > maximized, then restore the window to a smaller size, Vim goes blank
            > until the next progress bar update, and then the progress bar is too
            > large to fit on the screen and is truncated. This is not desirable,
            > but perhaps it would acceptable if the performance gains are great
            > enough. This does not seem to be the case, because I added back in the
            > size recalculation with no noticeable performance hit.
            >
            > The needs_redraw was done in order to allow us to call redrawstatus on
            > the correct window. :help redrawstatus says that it redraws the status
            > line for the *current window* only unless you use redrawstatus! which
            > redraws all windows. In practice, however, it does not seem to matter
            > which window we use it in. Why is this?
            >
            > > Also, why you forbid profiling progress bar functions? It is also
            > > fixed.
            >
            > Good catch, that's certainly something to include going forward.
            >
            > There is a slight speed gain from your patch, however there is a
            > mistaken assumption in the way you update the progress bar. Your code
            > assumes that the progress bar will only ever update by one tick at a
            > time. Updating the progress bar without your patch calculates the
            > entire string every time, using repeat(). Your update simply adds one
            > to the colored string of spaces, and subtracts one from the uncolored.
            > This does not work if the user folds away some text and does not use
            > dynamic folding, it does not work when there are fewer than 100 lines
            > in the text to convert, and it does not work for the second use of the
            > progress bar, where there are usually fewer that 100 highlight groups
            > to process.
            >
            > I corrected this problem and initially, the performance still seemed
            > to be improved over the previous version. However, I noticed afterward
            > that part of the patch removes the "sleep 100m" from the "processing
            > classes" step. I took this line out of the original script for a fair
            > comparison, and got the following timings, converting
            > autoload/netrw.vim (7764 lines) with dynamic folding enabled:
            >
            > Before patch: 50 seconds
            > Patch from ZyX: 49 seconds
            > Fixed patch: 51 seconds
            >
            > So, it looks like the patch is actually no faster, and potentially
            > slightly slower than the precalculated version.
            >
            > I have therefore attached an updated version of my last submission,
            > which removes floating point from the calculate_ticks function, and
            > incorporates some of the other improvements from ZyX.
            >
            > This version takes 50 seconds to convert netrw, if I comment out the
            > sleep 100 line. Do we want this line in the code? Without it, if there
            > are not very many highlight groups to process, the "processing
            > classes" bar flashes by without being seen. This happens anyway for
            > very small selections. I don't know how I feel about deliberately
            > slowing down the execution. I have left it commented out for now.
            >
            > I am very curious about this:
            >
            > " Note that you must use len(split) instead of len() if you want to use
            > " unicode in title
            > let self.pb_len = max_len-len(split(self.title, '\zs'))-3-4-2
            >
            > Can someone explain the problem described in the comment a little
            > better? And why does the split on '\zs' work to fix the problem?
            >
          • ZyX
            Ответ на сообщение «Re: Progress indicator for :TOhtml command», присланное в 13:03:23 06 июня 2010, Воскресенье,
            Message 5 of 28 , Jun 6, 2010
            • 0 Attachment
              Ответ на сообщение «Re: Progress indicator for :TOhtml command»,
              присланное в 13:03:23 06 июня 2010, Воскресенье,
              отправитель ZyX:

              Yes, buffer switching is the problem: attached patch uses my technique (save
              everything in a list, not in a buffer) and here are the results:

              My script:
              1:05,09 w/o progress
              1:08,40 ShowProgress=1
              1:20,59 ShowProgress=2
              Your 2html:
              1:19,67 w/o progress
              1:44,74 with progress
              Patched 2html:
              1:03,51 w/o progress
              1:05,08 with progress

              Apply patch to your 2html, not to previously patched version.

              Текст сообщения:
              > Ответ на сообщение «Re: Progress indicator for :TOhtml command»,
              > присланное в 10:59:42 06 июня 2010, Воскресенье,
              > отправитель Benjamin Fritz:
              >
              > It is odd: the only problem in your script is redrawstatus which is called
              > only 100 times (without styles, 109 with) (>21 seconds), while in my
              > script redrawstatus called 328 times takes less than a second.
              >
              > Second problem with the whole 2html is buffer switching, I think you should
              > consider instead of doing constant switches, save every line in a List and
              > only after everything is finished create a new buffer and call setline(1,
              > s:list). Note that new versions of my script are faster (but not much)
              > then your 2html because I use this technique.
              >
              > And, why do you calculate length of the title at each progressbarupdate?
              > Attached patch fixes this and the case when there is no space for progress
              > bar.
              >
              > Текст сообщения:
              > > On Jun 5, 8:10 pm, ZyX <zyx....@...> wrote:
              > > > It occures that the problem is not floating-point math: the attached
              > > > patch removes this math but does not add any perfomance.
              > >
              > > Yes, I did not expect any performance gains from removing the little
              > > bit of remaining floating point, since it is just up to 100
              > > calculations done once at the start and thereafter only when the
              > > window changes size. It is a good idea to remove, because as you point
              > > out, that amount of precision is probably unnecessary, and it would
              > > just introduce another dependency.
              > >
              > > > It also removes recalculating
              > > > progress bar width (you just used used some generic progress bar?) and
              > > > needs_redraw.
              > >
              > > Yes, we did use a generic progress bar as the starting point for this.
              > > However, I think it IS necessary to recalculate the progress bar
              > > width. This is done so that if the user changes window sizes, the
              > > progress bar will be updated accordingly. We don't want a progress bar
              > > that is too big to fit in the window, or smaller than needed for
              > > decent viewing. With your patch, if you start with the gvim window
              > > maximized, then restore the window to a smaller size, Vim goes blank
              > > until the next progress bar update, and then the progress bar is too
              > > large to fit on the screen and is truncated. This is not desirable,
              > > but perhaps it would acceptable if the performance gains are great
              > > enough. This does not seem to be the case, because I added back in the
              > > size recalculation with no noticeable performance hit.
              > >
              > > The needs_redraw was done in order to allow us to call redrawstatus on
              > > the correct window. :help redrawstatus says that it redraws the status
              > > line for the *current window* only unless you use redrawstatus! which
              > > redraws all windows. In practice, however, it does not seem to matter
              > > which window we use it in. Why is this?
              > >
              > > > Also, why you forbid profiling progress bar functions? It is also
              > > > fixed.
              > >
              > > Good catch, that's certainly something to include going forward.
              > >
              > > There is a slight speed gain from your patch, however there is a
              > > mistaken assumption in the way you update the progress bar. Your code
              > > assumes that the progress bar will only ever update by one tick at a
              > > time. Updating the progress bar without your patch calculates the
              > > entire string every time, using repeat(). Your update simply adds one
              > > to the colored string of spaces, and subtracts one from the uncolored.
              > > This does not work if the user folds away some text and does not use
              > > dynamic folding, it does not work when there are fewer than 100 lines
              > > in the text to convert, and it does not work for the second use of the
              > > progress bar, where there are usually fewer that 100 highlight groups
              > > to process.
              > >
              > > I corrected this problem and initially, the performance still seemed
              > > to be improved over the previous version. However, I noticed afterward
              > > that part of the patch removes the "sleep 100m" from the "processing
              > > classes" step. I took this line out of the original script for a fair
              > > comparison, and got the following timings, converting
              > > autoload/netrw.vim (7764 lines) with dynamic folding enabled:
              > >
              > > Before patch: 50 seconds
              > > Patch from ZyX: 49 seconds
              > > Fixed patch: 51 seconds
              > >
              > > So, it looks like the patch is actually no faster, and potentially
              > > slightly slower than the precalculated version.
              > >
              > > I have therefore attached an updated version of my last submission,
              > > which removes floating point from the calculate_ticks function, and
              > > incorporates some of the other improvements from ZyX.
              > >
              > > This version takes 50 seconds to convert netrw, if I comment out the
              > > sleep 100 line. Do we want this line in the code? Without it, if there
              > > are not very many highlight groups to process, the "processing
              > > classes" bar flashes by without being seen. This happens anyway for
              > > very small selections. I don't know how I feel about deliberately
              > > slowing down the execution. I have left it commented out for now.
              > >
              > > I am very curious about this:
              > >
              > > " Note that you must use len(split) instead of len() if you want to use
              > > " unicode in title
              > > let self.pb_len = max_len-len(split(self.title, '\zs'))-3-4-2
              > >
              > > Can someone explain the problem described in the comment a little
              > > better? And why does the split on '\zs' work to fix the problem?
              >
            • Benjamin Fritz
              ... Very nice. This is a huge performance boost, and the times are similar with and without the progress bar even with my big 33000 line C file which I used
              Message 6 of 28 , Jun 7, 2010
              • 0 Attachment
                On Sun, Jun 6, 2010 at 5:10 AM, ZyX <zyx.vim@...> wrote:
                >
                > Yes, buffer switching is the problem: attached patch uses my technique (save
                > everything in a list, not in a buffer) and here are the results:
                >
                > My script:
                > 1:05,09 w/o progress
                > 1:08,40 ShowProgress=1
                > 1:20,59 ShowProgress=2
                > Your 2html:
                > 1:19,67 w/o progress
                > 1:44,74 with progress
                > Patched 2html:
                > 1:03,51 w/o progress
                > 1:05,08 with progress
                >

                Very nice. This is a huge performance boost, and the times are similar
                with and without the progress bar even with my big 33000 line C file
                which I used previously.

                I think it's about ready now. I've added another progress bar for the
                time taken to collect fold information for dynamic folding, and
                corrected a few minor bugs in the patch related to dynamic folding. I
                did end up adding back in a :sleep to the class processing loop, but I
                reduced the time it sleeps. I'm certainly open to removing this.

                I've attached the whole file so we don't get into a "which patches do
                I need?" quagmire.

                --
                You received this message from the "vim_dev" maillist.
                Do not top-post! Type your reply below the text you are replying to.
                For more information, visit http://www.vim.org/maillist.php
              Your message has been successfully submitted and would be delivered to recipients shortly.