kot...@apache.org wrote on Fri, 14 Jul 2017 11:13 +0000:
> Author: kotkov
> Date: Fri Jul 14 11:13:47 2017
> New Revision: 1801940
>
> URL: http://svn.apache.org/viewvc?rev=1801940&view=rev
> Log:
> fsfs: Add initial support for LZ4 compression.
>
> This can significantly (up to 3 times) improve the speed of commits and
> other operations with large binary or incompressible files, while still
> maintaining a decent compression ratio.
> From the client perspective, the patch starts using LZ4 compression
> only for file:// protocol, and the support/negotiation of the use of svndiff2
> with LZ4 compression for http:// and svn:// can be added later.
To be clear, ra_svn in current trunk is interoperable with 1.9, right?
I.e., it doesn't use svndiff2 over the wire.
> With this patch, LZ4 compression will be enabled for fsfs repositories which
> specify compression-level=1 in fsfs.conf.
>
> +++ subversion/trunk/subversion/libsvn_fs_fs/transaction.c Fri Jul 14
> 11:13:47 2017
> @@ -2159,6 +2159,38 @@ rep_write_cleanup(void *data)
> return APR_SUCCESS;
> }
>
> +static void
> +txdelta_to_svndiff(svn_txdelta_window_handler_t *handler,
> + void **handler_baton,
> + svn_stream_t *output,
> + svn_fs_t *fs,
> + apr_pool_t *pool)
> +{
> + fs_fs_data_t *ffd = fs->fsap_data;
> + int svndiff_version;
> + int svndiff_compression_level;
> +
> + if (ffd->delta_compression_level == 1 &&
> + ffd->format >= SVN_FS_FS__MIN_SVNDIFF2_FORMAT)
> + {
> + svndiff_version = 2;
> + svndiff_compression_level = 0;
> + }
> + else if (ffd->format >= SVN_FS_FS__MIN_SVNDIFF1_FORMAT)
> + {
> + svndiff_version = 1;
> + svndiff_compression_level = ffd->delta_compression_level;
> + }
> + else
> + {
> + svndiff_version = 0;
> + svndiff_compression_level = 0;
> + }
> +
> + svn_txdelta_to_svndiff3(handler, handler_baton, output, svndiff_version,
> + svndiff_compression_level, pool);
> +}
I'm a bit uncomfortable with this logic.
1. It violates the principle of least surprise: compression-level=9
means 'gzip -9', compression-level=5 means 'gzip -5', but
compression-level=1 means LZ4 (with the default acceleration_factor)
rather than 'gzip -1'.
2. It leaves no way to use zlib level 1 in f8 filesystems. This seems
like a decision that should be left to the admin, rather than hardcoded
into the library.
3. What if somebody wanted to add a backend with, say, xz compression.
(xz compression also takes levels like gzip.) Would it make sense to have
two tunables:
.
compression-backend = { lz4 | zlib }
compression-level = {1..N for lz4, 1..9 for zlib}
.
and then other compression backends could be easily added?
This would also allow admins to set the 'acceleration_factor' of lz4.
> The interoperability is implemented by bumping the format of svndiff
> to 2 and the repository file system format to 8.
Cheers,
Daniel
