You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2010/08/02 22:51:35 UTC
svn commit: r981684 - in
/subversion/branches/performance/subversion/libsvn_fs_fs: caching.c fs_fs.h
Author: stefan2
Date: Mon Aug 2 20:51:35 2010
New Revision: 981684
URL: http://svn.apache.org/viewvc?rev=981684&view=rev
Log:
Bring the membuffer cache to its first use for the full text cache.
Also, provide functions to get / set the FSFS cache configuration
although not all of it is supported, yet.
* subversion/libsvn_fs_fs/fs_fs.h
(svn_fs_fs__cache_config_t): introduce this new config struct
(svn_fs_fs__get_cache_config, svn_fs_fs__set_cache_config):
declare new functions to get/set the FSFS cache configuration
* subversion/libsvn_fs_fs/caching.c
(internal_get_cache_config, get_global_membuffer_cache):
new singleton management functions
(svn_fs_fs__get_cache_config, svn_fs_fs__set_cache_config):
implement new functions to get/set the FSFS cache configuration
(svn_fs_fs__initialize_caches): use the membuffer cache for
full texts if memcached has not been configured
Modified:
subversion/branches/performance/subversion/libsvn_fs_fs/caching.c
subversion/branches/performance/subversion/libsvn_fs_fs/fs_fs.h
Modified: subversion/branches/performance/subversion/libsvn_fs_fs/caching.c
URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_fs_fs/caching.c?rev=981684&r1=981683&r2=981684&view=diff
==============================================================================
--- subversion/branches/performance/subversion/libsvn_fs_fs/caching.c (original)
+++ subversion/branches/performance/subversion/libsvn_fs_fs/caching.c Mon Aug 2 20:51:35 2010
@@ -27,6 +27,7 @@
#include "../libsvn_fs/fs-loader.h"
#include "svn_config.h"
+#include "svn_pools.h"
#include "svn_private_config.h"
@@ -189,6 +190,80 @@ warn_on_cache_errors(svn_error_t *err,
return SVN_NO_ERROR;
}
+/* Access to the cache settings as a process-wide singleton. */
+static svn_fs_fs__cache_config_t *
+internal_get_cache_config(void)
+{
+ static svn_fs_fs__cache_config_t settings =
+ {
+ /* default configuration:
+ */
+ 0x8000000, /* 128 MB for caches */
+ 16, /* up to 16 files kept open */
+ FALSE, /* don't cache fulltexts */
+ FALSE, /* don't cache text deltas */
+ FALSE /* assume multi-threaded operation */
+ };
+
+ return &settings;
+}
+
+/* Get the current FSFS cache configuration. */
+const svn_fs_fs__cache_config_t *
+svn_fs_fs__get_cache_config(void)
+{
+ return internal_get_cache_config();
+}
+
+/* Access the process-global (singleton) membuffer cache. The first call
+ * will automatically allocate the cache using the current cache config.
+ * NULL will be returned if the desired cache size is 0.
+ */
+static svn_membuffer_t *
+get_global_membuffer_cache(void)
+{
+ static svn_membuffer_t *cache = NULL;
+
+ apr_uint64_t cache_size = svn_fs_fs__get_cache_config()->cache_size;
+ if (!cache && cache_size)
+ {
+ /* auto-allocate cache*/
+ apr_allocator_t *allocator = NULL;
+ apr_pool_t *pool = NULL;
+
+ if (apr_allocator_create(&allocator))
+ return NULL;
+
+ /* ensure that a we free partially allocated data if we run OOM
+ * before the cache is complete.
+ */
+ apr_allocator_max_free_set(allocator, 1);
+ pool = svn_pool_create_ex(NULL, allocator);
+
+ svn_cache__membuffer_cache_create
+ (&cache,
+ cache_size,
+ cache_size / 16,
+ ! svn_fs_fs__get_cache_config()->single_threaded,
+ pool);
+ }
+
+ return cache;
+}
+
+/* Set the current FSFS cache configuration. Apply it immediately to ensure
+ * thread-safety: since this function should be called from the processes'
+ * initialization code, no race with background / worker threads should occur.
+ */
+void
+svn_fs_fs__set_cache_config(svn_fs_fs__cache_config_t *settings)
+{
+ *internal_get_cache_config() = *settings;
+
+ /* Allocate global membuffer cache as a side-effect.
+ * Only the first call will actually take affect. */
+ get_global_membuffer_cache();
+}
svn_error_t *
svn_fs_fs__initialize_caches(svn_fs_t *fs,
@@ -300,12 +375,27 @@ svn_fs_fs__initialize_caches(svn_fs_t *f
apr_pstrcat(pool, prefix, "TEXT",
NULL),
fs->pool));
- if (! no_handler)
- SVN_ERR(svn_cache__set_error_handler(ffd->fulltext_cache,
- warn_on_cache_errors, fs, pool));
+ }
+ else if (get_global_membuffer_cache() &&
+ svn_fs_fs__get_cache_config()->cache_fulltexts)
+ {
+ SVN_ERR(svn_cache__create_membuffer_cache(&(ffd->fulltext_cache),
+ get_global_membuffer_cache(),
+ /* Values are svn_string_t */
+ NULL, NULL,
+ APR_HASH_KEY_STRING,
+ apr_pstrcat(pool, prefix, "TEXT",
+ NULL),
+ fs->pool));
}
else
- ffd->fulltext_cache = NULL;
+ {
+ ffd->fulltext_cache = NULL;
+ }
+
+ if (ffd->fulltext_cache && ! no_handler)
+ SVN_ERR(svn_cache__set_error_handler(ffd->fulltext_cache,
+ warn_on_cache_errors, fs, pool));
return SVN_NO_ERROR;
}
Modified: subversion/branches/performance/subversion/libsvn_fs_fs/fs_fs.h
URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_fs_fs/fs_fs.h?rev=981684&r1=981683&r2=981684&view=diff
==============================================================================
--- subversion/branches/performance/subversion/libsvn_fs_fs/fs_fs.h (original)
+++ subversion/branches/performance/subversion/libsvn_fs_fs/fs_fs.h Mon Aug 2 20:51:35 2010
@@ -523,6 +523,39 @@ svn_fs_fs__get_node_origin(const svn_fs_
svn_error_t *
svn_fs_fs__initialize_caches(svn_fs_t *fs, apr_pool_t *pool);
+/* FSFS cache settings. It controls what caches, in what size and
+ how they will be created. The settomgs apply for the whole process.
+ */
+typedef struct svn_fs_fs__cache_config_t
+{
+ /* total cache size in bytes. May be 0, resulting in no cache being used */
+ apr_uint64_t cache_size;
+
+ /* maximum number of files kept open */
+ apr_size_t file_handle_count;
+
+ /* shall fulltexts be cached? */
+ svn_boolean_t cache_fulltexts;
+
+ /* shall text deltas be cached? */
+ svn_boolean_t cache_txdeltas;
+
+ /* is this a guaranteed single-threaded application? */
+ svn_boolean_t single_threaded;
+} svn_fs_fs__cache_config_t;
+
+/* Get the current FSFS cache configuration. If it has not been set,
+ yet, this function will return the default settings.
+ */
+const svn_fs_fs__cache_config_t *
+svn_fs_fs__get_cache_config(void);
+
+/* Set the FSFS cache configuration. Please note that it may not change
+ the actual configuration *in use*. Therefore, call it before reading
+ data from any FSFS repo and call it only once.
+ */
+void
+svn_fs_fs__set_cache_config(svn_fs_fs__cache_config_t *settings);
/* Possibly pack the repository at PATH. This just take full shards, and
combines all the revision files into a single one, with a manifest header.
Re: svn commit: r981684 - in /subversion/branches/performance/subversion/libsvn_fs_fs:
caching.c fs_fs.h
Posted by Stefan Fuhrmann <st...@alice-dsl.de>.
Blair Zajac wrote:
> On 08/02/2010 01:51 PM, stefan2@apache.org wrote:
>> Author: stefan2
>> Date: Mon Aug 2 20:51:35 2010
>> New Revision: 981684
>>
>> URL: http://svn.apache.org/viewvc?rev=981684&view=rev
>> Log:
>> Bring the membuffer cache to its first use for the full text cache.
>> Also, provide functions to get / set the FSFS cache configuration
>> although not all of it is supported, yet.
>>
>
> Stefan,
>
> I appreciate you working on this, I'm looking forward to using these
> improvements in production, which is why I'm closely looking at them.
Hi Blair,
I'm always open to constructive criticism.
So, tanks for your feedback!
>
>> +/* Access to the cache settings as a process-wide singleton. */
>> +static svn_fs_fs__cache_config_t *
>> +internal_get_cache_config(void)
>> +{
>> + static svn_fs_fs__cache_config_t settings =
>> + {
>> + /* default configuration:
>> + */
>> + 0x8000000, /* 128 MB for caches */
>> + 16, /* up to 16 files kept open */
>> + FALSE, /* don't cache fulltexts */
>> + FALSE, /* don't cache text deltas */
>> + FALSE /* assume multi-threaded operation */
>> + };
>> +
>> + return&settings;
>> +}
>
>
> Why not make "settings" static at file scope instead of inside this
> function? What does this function provide for us?
Just a habit of mine from the C++ world (Meyer's singleton).
But since this is a POD structure, there should be no
initialization issues. I changed it to a static variable.
>
>> +/* Access the process-global (singleton) membuffer cache. The first
>> call
>> + * will automatically allocate the cache using the current cache
>> config.
>> + * NULL will be returned if the desired cache size is 0.
>> + */
>> +static svn_membuffer_t *
>> +get_global_membuffer_cache(void)
>> +{
>> + static svn_membuffer_t *cache = NULL;
>> +
>> + apr_uint64_t cache_size = svn_fs_fs__get_cache_config()->cache_size;
>> + if (!cache&& cache_size)
>
> s/cache&&/cache &&/
This seems to be a mailer artifact. At least in my editor it shows up fine.
>
>> + {
>> + /* auto-allocate cache*/
>> + apr_allocator_t *allocator = NULL;
>> + apr_pool_t *pool = NULL;
>> +
>> + if (apr_allocator_create(&allocator))
>> + return NULL;
>> +
>> + /* ensure that a we free partially allocated data if we run OOM
>
>
> s/a we/we/
Fixed.
>> + */
>> + apr_allocator_max_free_set(allocator, 1);
>> + pool = svn_pool_create_ex(NULL, allocator);
>> +
>> + svn_cache__membuffer_cache_create
>> + (&cache,
>> + cache_size,
>> + cache_size / 16,
>> + ! svn_fs_fs__get_cache_config()->single_threaded,
>> + pool);
>> + }
> + * before the cache is complete.
>
> So the allocator is associated with this pool forever then? So it
> cannot be destroyed after
It limits the amount of *unused* memory held by the pool to 1 byte.
I extended on the commentary how that helps with certain OOM
situations.
>> + {
>> + ffd->fulltext_cache = NULL;
>> + }
>> +
>> + if (ffd->fulltext_cache&& ! no_handler)
> - ffd->fulltext_cache = NULL;
>
> s/fulltext_cache&& ! no_handler/fulltext_cache && ! no_handler
Same artifact as above.
>
>> + SVN_ERR(svn_cache__set_error_handler(ffd->fulltext_cache,
>> + warn_on_cache_errors, fs, pool));
>>
>> return SVN_NO_ERROR;
>> }
>>
>> Modified:
>> subversion/branches/performance/subversion/libsvn_fs_fs/fs_fs.h
>> URL:
>> http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_fs_fs/fs_fs.h?rev=981684&r1=981683&r2=981684&view=diff
>>
>> ==============================================================================
>>
>> --- subversion/branches/performance/subversion/libsvn_fs_fs/fs_fs.h
>> (original)
>> +++ subversion/branches/performance/subversion/libsvn_fs_fs/fs_fs.h
>> Mon Aug 2 20:51:35 2010
>> @@ -523,6 +523,39 @@ svn_fs_fs__get_node_origin(const svn_fs_
>> svn_error_t *
>> svn_fs_fs__initialize_caches(svn_fs_t *fs, apr_pool_t *pool);
>>
>> +/* FSFS cache settings. It controls what caches, in what size and
>> + how they will be created. The settomgs apply for the whole process.
>
> s/settomgs/settings/
Fixed.
>
>> + */
>> +typedef struct svn_fs_fs__cache_config_t
>> +{
>> + /* total cache size in bytes. May be 0, resulting in no cache
>> being used */
>> + apr_uint64_t cache_size;
>> +
>> + /* maximum number of files kept open */
>> + apr_size_t file_handle_count;
>> +
>> + /* shall fulltexts be cached? */
>> + svn_boolean_t cache_fulltexts;
>> +
>> + /* shall text deltas be cached? */
>> + svn_boolean_t cache_txdeltas;
>> +
>> + /* is this a guaranteed single-threaded application? */
>> + svn_boolean_t single_threaded;
>> +} svn_fs_fs__cache_config_t;
>> +
>> +/* Get the current FSFS cache configuration. If it has not been set,
>> + yet, this function will return the default settings.
>
>
> s/yet,//
dito.
>
>> + */
>> +const svn_fs_fs__cache_config_t *
>> +svn_fs_fs__get_cache_config(void);
>> +
>> +/* Set the FSFS cache configuration. Please note that it may not change
>> + the actual configuration *in use*. Therefore, call it before reading
>> + data from any FSFS repo and call it only once.
>
> Should also document that this is not multi-threaded safe and does not
> protect against races, just as you do at the functions implementation.
done.
>> +void
>> +svn_fs_fs__set_cache_config(svn_fs_fs__cache_config_t *settings);
> + */
>
> Instead of
>
> svn_fs_fs__cache_config_t *settings
>
> make it const:
>
> const svn_fs_fs__cache_config_t *settings
done.
>
> Regards,
> Blair
>
-- Stefan^2.
Re: svn commit: r981684 - in /subversion/branches/performance/subversion/libsvn_fs_fs:
caching.c fs_fs.h
Posted by Blair Zajac <bl...@orcaware.com>.
On 08/02/2010 01:51 PM, stefan2@apache.org wrote:
> Author: stefan2
> Date: Mon Aug 2 20:51:35 2010
> New Revision: 981684
>
> URL: http://svn.apache.org/viewvc?rev=981684&view=rev
> Log:
> Bring the membuffer cache to its first use for the full text cache.
> Also, provide functions to get / set the FSFS cache configuration
> although not all of it is supported, yet.
>
Stefan,
I appreciate you working on this, I'm looking forward to using these
improvements in production, which is why I'm closely looking at them.
> +/* Access to the cache settings as a process-wide singleton. */
> +static svn_fs_fs__cache_config_t *
> +internal_get_cache_config(void)
> +{
> + static svn_fs_fs__cache_config_t settings =
> + {
> + /* default configuration:
> + */
> + 0x8000000, /* 128 MB for caches */
> + 16, /* up to 16 files kept open */
> + FALSE, /* don't cache fulltexts */
> + FALSE, /* don't cache text deltas */
> + FALSE /* assume multi-threaded operation */
> + };
> +
> + return&settings;
> +}
Why not make "settings" static at file scope instead of inside this
function? What does this function provide for us?
> +/* Access the process-global (singleton) membuffer cache. The first call
> + * will automatically allocate the cache using the current cache config.
> + * NULL will be returned if the desired cache size is 0.
> + */
> +static svn_membuffer_t *
> +get_global_membuffer_cache(void)
> +{
> + static svn_membuffer_t *cache = NULL;
> +
> + apr_uint64_t cache_size = svn_fs_fs__get_cache_config()->cache_size;
> + if (!cache&& cache_size)
s/cache&&/cache &&/
> + {
> + /* auto-allocate cache*/
> + apr_allocator_t *allocator = NULL;
> + apr_pool_t *pool = NULL;
> +
> + if (apr_allocator_create(&allocator))
> + return NULL;
> +
> + /* ensure that a we free partially allocated data if we run OOM
s/a we/we/
> + * before the cache is complete.
> + */
> + apr_allocator_max_free_set(allocator, 1);
> + pool = svn_pool_create_ex(NULL, allocator);
> +
> + svn_cache__membuffer_cache_create
> + (&cache,
> + cache_size,
> + cache_size / 16,
> + ! svn_fs_fs__get_cache_config()->single_threaded,
> + pool);
> + }
So the allocator is associated with this pool forever then? So it
cannot be destroyed after
> - ffd->fulltext_cache = NULL;
> + {
> + ffd->fulltext_cache = NULL;
> + }
> +
> + if (ffd->fulltext_cache&& ! no_handler)
s/fulltext_cache&& ! no_handler/fulltext_cache && ! no_handler
> + SVN_ERR(svn_cache__set_error_handler(ffd->fulltext_cache,
> + warn_on_cache_errors, fs, pool));
>
> return SVN_NO_ERROR;
> }
>
> Modified: subversion/branches/performance/subversion/libsvn_fs_fs/fs_fs.h
> URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_fs_fs/fs_fs.h?rev=981684&r1=981683&r2=981684&view=diff
> ==============================================================================
> --- subversion/branches/performance/subversion/libsvn_fs_fs/fs_fs.h (original)
> +++ subversion/branches/performance/subversion/libsvn_fs_fs/fs_fs.h Mon Aug 2 20:51:35 2010
> @@ -523,6 +523,39 @@ svn_fs_fs__get_node_origin(const svn_fs_
> svn_error_t *
> svn_fs_fs__initialize_caches(svn_fs_t *fs, apr_pool_t *pool);
>
> +/* FSFS cache settings. It controls what caches, in what size and
> + how they will be created. The settomgs apply for the whole process.
s/settomgs/settings/
> + */
> +typedef struct svn_fs_fs__cache_config_t
> +{
> + /* total cache size in bytes. May be 0, resulting in no cache being used */
> + apr_uint64_t cache_size;
> +
> + /* maximum number of files kept open */
> + apr_size_t file_handle_count;
> +
> + /* shall fulltexts be cached? */
> + svn_boolean_t cache_fulltexts;
> +
> + /* shall text deltas be cached? */
> + svn_boolean_t cache_txdeltas;
> +
> + /* is this a guaranteed single-threaded application? */
> + svn_boolean_t single_threaded;
> +} svn_fs_fs__cache_config_t;
> +
> +/* Get the current FSFS cache configuration. If it has not been set,
> + yet, this function will return the default settings.
s/yet,//
> + */
> +const svn_fs_fs__cache_config_t *
> +svn_fs_fs__get_cache_config(void);
> +
> +/* Set the FSFS cache configuration. Please note that it may not change
> + the actual configuration *in use*. Therefore, call it before reading
> + data from any FSFS repo and call it only once.
Should also document that this is not multi-threaded safe and does not
protect against races, just as you do at the functions implementation.
> + */
> +void
> +svn_fs_fs__set_cache_config(svn_fs_fs__cache_config_t *settings);
Instead of
svn_fs_fs__cache_config_t *settings
make it const:
const svn_fs_fs__cache_config_t *settings
Regards,
Blair