Blair Zajac wrote:
On 08/02/2010 01:51 PM, [email protected] 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.
