Re: svn commit: r1666965 - /subversion/trunk/subversion/mod_dav_svn/reports/log.c

[Branko Čibej <br...@wandisco.com>]

On 16.03.2015 12:30, rhuijben@apache.org wrote:
> Author: rhuijben
> Date: Mon Mar 16 11:30:04 2015
> New Revision: 1666965
>
> URL: http://svn.apache.org/r1666965
> Log:
> Reduce the perceived very slow operation of svn log URL via mod_dav on
> large result sets that return not that much data, by flushing the
> httpd output cache a few times for the 'first few log results'.
[...]
> +  /* In general APR will flush the bucket every 8 KByte, but log items
> +     may not be generated that fast, especially in combination with authz
> +     and busy servers. This algorithm makes sure the client gets the first
> +     two results very fast (but less efficient), while it gradually removes
> +     its performance hit and falls back to the APR standard buffer handling,
> +     which streamlines the http processing */
> +  lrb->result_count++;
> +  if (lrb->result_count == lrb->next_forced_flush)
> +    {
> +      apr_off_t len = 0;
> +      (void)apr_brigade_length(lrb->bb, FALSE, &len);
> +
> +      if (len != 0)
> +        {
> +          apr_status_t apr_err = ap_fflush(lrb->output, lrb->bb);
> +          if (apr_err)
> +            return svn_error_create(apr_err, NULL, NULL);
> +
> +          if (lrb->output->c->aborted)
> +            return svn_error_create(SVN_ERR_APMOD_CONNECTION_ABORTED,
> +                                    NULL, NULL);
> +        }
> +
> +      if (lrb->result_count < 2048)
> +        lrb->next_forced_flush = lrb->next_forced_flush * 2;
> +    }
> +
>    return SVN_NO_ERROR;
>  }

I'd like to suggest a slightly different flushing policy:

--- log.c    (revision 1667032)
+++ log.c    (working copy)
@@ -68,6 +68,7 @@ struct log_receiver_baton 
   /* Helper variables to force early bucket brigade flushes */
   int result_count;
+  int forced_flush_interval;
   int next_forced_flush;
 }; 
@@ -310,7 +311,11 @@ log_receiver(void *baton,
         } 
       if (lrb->result_count < 2048)
-        lrb->next_forced_flush = lrb->next_forced_flush * 2;
+        {
+          lrb->forced_flush_interval *= 2;
+          lrb->next_forced_flush =
+            lrb->result_count + lrb->forced_flush_interval;
+        }
     } 
   return SVN_NO_ERROR;
@@ -459,7 +464,7 @@ dav_svn__log_report(const dav_resource *resource,
   /* lrb.requested_custom_revprops set above */ 
   lrb.result_count = 0;
-  lrb.next_forced_flush = 1;
+  lrb.forced_flush_interval = lrb.next_forced_flush = 1;

   /* Our svn_log_entry_receiver_t sends the <S:log-report> header in
      a lazy fashion.  Before writing the first log message, it assures


In other words, double the interval between successive flushes; instead
of flushing at results 1, 2, 4, 8, 16 ... this will flush at 1, 3, 7,
15, 31 ... which will result in half as many forced flushes without a
significant difference in the perceived frequency of the first few results.

-- Brane

Re: svn commit: r1666965 - /subversion/trunk/subversion/mod_dav_svn/reports/log.c

[Julian Foad <ju...@btopenworld.com>]

Branko Čibej wrote:
> I'd like to suggest a slightly different flushing policy:
>
> --- log.c    (revision 1667032)
> +++ log.c    (working copy)
> @@ -68,6 +68,7 @@ struct log_receiver_baton
>    /* Helper variables to force early bucket brigade flushes */
>    int result_count;
> +  int forced_flush_interval;
>    int next_forced_flush;
>  };
> @@ -310,7 +311,11 @@ log_receiver(void *baton,
>          }
>        if (lrb->result_count < 2048)
> -        lrb->next_forced_flush = lrb->next_forced_flush * 2;
> +        {
> +          lrb->forced_flush_interval *= 2;
> +          lrb->next_forced_flush =
> +            lrb->result_count + lrb->forced_flush_interval;
> +        }
>      }
>    return SVN_NO_ERROR;
> @@ -459,7 +464,7 @@ dav_svn__log_report(const dav_resource *resource,
>    /* lrb.requested_custom_revprops set above */
>    lrb.result_count = 0;
> -  lrb.next_forced_flush = 1;
> +  lrb.forced_flush_interval = lrb.next_forced_flush = 1;
>
>    /* Our svn_log_entry_receiver_t sends the <S:log-report> header in
>       a lazy fashion.  Before writing the first log message, it assures
>
>
> In other words, double the interval between successive flushes; instead
> of flushing at results 1, 2, 4, 8, 16 ... this will flush at 1, 3, 7,
> 15, 31 ... which will result in half as many forced flushes without a
> significant difference in the perceived frequency of the first few results.

That's *one fewer* flush, not half as many!

Make the multiplier bigger -- 3 or 4 instead of 2 -- to get fewer
flushes while still exponential. But see my plea in the other email
thread for trying to develop a time-based flushing instead (or perhaps
it could be in combination with counting).

- Julian