You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Berlin <db...@dberlin.org> on 2005/10/20 17:08:28 UTC
[PATCH]: Stop putting stuff in empty props files
On gcc checkouts, empty prop files with just the word END in them take
up roughly 250 meg of space.
I'm not joking.
This is simple to fix, however. We just need to start using hash_write2
with a NULL terminator in the wc code, to make them completely empty,
instead of using the old hash_read/hash_write.
However, this is not backwards compatible with hash_read.
I would really want to be able to get this into 1.3.x somehow, since
this is a very very big issue for gcc (it takes our working copy size
from 960 meg to 620, while cvs is 340).
I'm working on the auto-upgrade code to make this auto-upgrade okay (the
autoupgrade code just needs to walk all the props files once and rewrite
them), but i wanted some feedback about whether it's possible to get
into 1.3.x before i finish that.
[[[
Don't terminate empty props files with END
* subversion/libsvn_wc/props.c (svn_wc__load_prop_file): Use hash_read2
with no terminator.
(svn_wc__save_prop_file): Use hash_write2 with no terminator.
(svn_wc__wcprop_set): Ditto.
(svn_wc_prop_set_2): Ditto
]]]
Re: [PATCH]: Stop putting stuff in empty props files
Posted by Ivan Zhakov <ch...@gmail.com>.
On 10/21/05, Daniel Berlin <db...@dberlin.org> wrote:
> On Thu, 2005-10-20 at 19:32 +0100, Philip Martin wrote:
> > Daniel Berlin <db...@dberlin.org> writes:
> >
> > > On gcc checkouts, empty prop files with just the word END in them take
> > > up roughly 250 meg of space.
> >
> > That's a known problem, one possible solution is described in
> > notes/wc-improvements. Implementation has got as far as creating a
> > wc-propcaching branch
> >
> I may look into this for 1.4 more hevaily, as i want to further reduce
> our working copy size (yes, i know about maxb's plans, etc :P)
>
>
> > > I'm not joking.
> > >
> > > This is simple to fix, however. We just need to start using hash_write2
> > > with a NULL terminator in the wc code, to make them completely empty,
> > > instead of using the old hash_read/hash_write.
> >
> > You are relying on an empty file not allocating a single disk block,
> > yes?
>
> Yes.
>
> > > However, this is not backwards compatible with hash_read.
> > >
> > > I would really want to be able to get this into 1.3.x somehow, since
> > > this is a very very big issue for gcc (it takes our working copy size
> > > from 960 meg to 620, while cvs is 340).
> >
> > You might be disappointed, last time I looked at the test gcc
> > repository it used both svn:ignore and svn:eol-style, there was only
> > one item with no properties.
>
>
> I follow the "Trust, but verify" model before I produce numbers :)
>
> checkout from 1.2.x on reiser
> 889556 gcc
>
> checkout from patched 1.3.x on reiser:
> 641968 gcc
>
I already have patch sitting in my working copy that removes prop
files if there is no props. I have tested it with trunk. Do you want
try it for compability?
--
Ivan Zhakov
Re: [PATCH]: Stop putting stuff in empty props files
Posted by Daniel Berlin <db...@dberlin.org>.
On Thu, 2005-10-20 at 19:32 +0100, Philip Martin wrote:
> Daniel Berlin <db...@dberlin.org> writes:
>
> > On gcc checkouts, empty prop files with just the word END in them take
> > up roughly 250 meg of space.
>
> That's a known problem, one possible solution is described in
> notes/wc-improvements. Implementation has got as far as creating a
> wc-propcaching branch
>
I may look into this for 1.4 more hevaily, as i want to further reduce
our working copy size (yes, i know about maxb's plans, etc :P)
> > I'm not joking.
> >
> > This is simple to fix, however. We just need to start using hash_write2
> > with a NULL terminator in the wc code, to make them completely empty,
> > instead of using the old hash_read/hash_write.
>
> You are relying on an empty file not allocating a single disk block,
> yes?
Yes.
> > However, this is not backwards compatible with hash_read.
> >
> > I would really want to be able to get this into 1.3.x somehow, since
> > this is a very very big issue for gcc (it takes our working copy size
> > from 960 meg to 620, while cvs is 340).
>
> You might be disappointed, last time I looked at the test gcc
> repository it used both svn:ignore and svn:eol-style, there was only
> one item with no properties.
I follow the "Trust, but verify" model before I produce numbers :)
checkout from 1.2.x on reiser
889556 gcc
checkout from patched 1.3.x on reiser:
641968 gcc
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Stop putting stuff in empty props files
Posted by Philip Martin <ph...@codematters.co.uk>.
Daniel Berlin <db...@dberlin.org> writes:
> On gcc checkouts, empty prop files with just the word END in them take
> up roughly 250 meg of space.
That's a known problem, one possible solution is described in
notes/wc-improvements. Implementation has got as far as creating a
wc-propcaching branch
> I'm not joking.
>
> This is simple to fix, however. We just need to start using hash_write2
> with a NULL terminator in the wc code, to make them completely empty,
> instead of using the old hash_read/hash_write.
You are relying on an empty file not allocating a single disk block,
yes? That sounds like a reasonable optimisation.
> However, this is not backwards compatible with hash_read.
>
> I would really want to be able to get this into 1.3.x somehow, since
> this is a very very big issue for gcc (it takes our working copy size
> from 960 meg to 620, while cvs is 340).
You might be disappointed, last time I looked at the test gcc
repository it used both svn:ignore and svn:eol-style, there was only
one item with no properties.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Stop putting stuff in empty props files
Posted by Branko Čibej <br...@xbc.nu>.
Daniel Berlin wrote:
>>Aieee, bad indentatin in log message!
>>
>>
>
>I have revised history to reflect a better indented log
>:)
>
>
>
>>Aargh, extraneous new line with two trailing spaces!
>>
>>
>>
>
>This one i caught before the commit on my own :)
>
>
>
>>Disregarding my deep and relevant comments above, if this truly doesn't
>>cause an incompatibility and/or doesn't require a WC version update,
>>
>>
>
>I did the following:
>
>1. Checked out a url with 1.2.x
> Tried newly patched 1.3.x with this wc
>2. Checked out a url with 1.3.x (not patched)
> Tried newly patched 1.3.x with this wc
>3. Checked out a url with newly patched 1.3.x
> Tried non-patched 1.2.x with this wc
> Tried non-patched 1.3.x with this wc
>
>All work with both some and no properties.
>
>
Good. +1 for the backport to 1.3 then. Ah! I see it's already in the
status file. r16862, then.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Stop putting stuff in empty props files
Posted by Daniel Berlin <db...@dberlin.org>.
> >
> Aieee, bad indentatin in log message!
I have revised history to reflect a better indented log
:)
> Aargh, extraneous new line with two trailing spaces!
>
This one i caught before the commit on my own :)
>
> Disregarding my deep and relevant comments above, if this truly doesn't
> cause an incompatibility and/or doesn't require a WC version update,
I did the following:
1. Checked out a url with 1.2.x
Tried newly patched 1.3.x with this wc
2. Checked out a url with 1.3.x (not patched)
Tried newly patched 1.3.x with this wc
3. Checked out a url with newly patched 1.3.x
Tried non-patched 1.2.x with this wc
Tried non-patched 1.3.x with this wc
All work with both some and no properties.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Stop putting stuff in empty props files
Posted by Branko Čibej <br...@xbc.nu>.
Daniel Berlin wrote:
>On Thu, 2005-10-20 at 13:15 -0400, Daniel Berlin wrote:
>
>
>>Actually, i think i can teach it to not create the props file if the
>>hash is empty, which works on all versions (they stat the path first,
>>and treat non-existence as emptyness).
>>
>>That works fine for my porpoises.
>>
>>
>
>Yup, the attached actually works.
>
>This is fully backwards compatible with 1.2.x due to how hash_read
>works.
>
>Again, i'd badly like this change in 1.3.x.
>
>(In 1.4, we probably want to not create the file at all. It turns out
>not everything uses empty_props_p, however, so this actually introduces
>an incompatibility)
>
>For the curious, gcc's checkout has 30k files in it
>
>each 4 byte file was taking 1 disk block of 4096 bytes.
>
>30k * 4k = 120 meg
>
>multiply by 2 (one props, one props-base), and you get 240 meg.
>
>Whee.
>
>This also stops useless I/O when a file has no properties set, as a
>side-benefit.
>
>I've applied this to the trunk in the meanwhile.
>
>[[[
>
>Don't write empty hashes to the props file, it just wastes space.
>
>* subversion/libsvn_wc/props.c (svn_wc__save_prop_file): Check hash
>count before writing to file.
>(empty_props_p): finfo.size == 0 is empty too.
>
>
Aieee, bad indentatin in log message!
>Index: subversion/libsvn_wc/props.c
>===================================================================
>--- subversion/libsvn_wc/props.c (revision 16853)
>+++ subversion/libsvn_wc/props.c (working copy)
>@@ -187,16 +187,18 @@
> apr_pool_t *pool)
> {
> apr_file_t *prop_tmp;
>+
>
>
Aargh, extraneous new line with two trailing spaces!
>
> SVN_ERR (svn_io_file_open (&prop_tmp, propfile_path,
> (APR_WRITE | APR_CREATE | APR_TRUNCATE
> | APR_BUFFERED),
> APR_OS_DEFAULT, pool));
>
>- SVN_ERR_W (svn_hash_write (hash, prop_tmp, pool),
>- apr_psprintf (pool,
>- _("Can't write property hash to '%s'"),
>- svn_path_local_style (propfile_path, pool)));
>+ if (apr_hash_count (hash) != 0)
>+ SVN_ERR_W (svn_hash_write (hash, prop_tmp, pool),
>+ apr_psprintf (pool,
>+ _("Can't write property hash to '%s'"),
>+ svn_path_local_style (propfile_path, pool)));
>
> SVN_ERR (svn_io_file_close (prop_tmp, pool));
>
>@@ -1729,7 +1731,7 @@
>
> /* If we remove props from a propfile, eventually the file will
> contain nothing but "END\n" */
>- if (finfo.filetype == APR_REG && finfo.size == 4)
>+ if (finfo.filetype == APR_REG && (finfo.size == 4 || finfo.size == 0))
> *empty_p = TRUE;
>
>
Heh. :)
Disregarding my deep and relevant comments above, if this truly doesn't
cause an incompatibility and/or doesn't require a WC version update,
then please commit and add my +1 to the backport entry in the 1.3.x
STATUS file.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Stop putting stuff in empty props files
Posted by Daniel Berlin <db...@dberlin.org>.
On Thu, 2005-10-20 at 13:15 -0400, Daniel Berlin wrote:
> Actually, i think i can teach it to not create the props file if the
> hash is empty, which works on all versions (they stat the path first,
> and treat non-existence as emptyness).
>
> That works fine for my porpoises.
Yup, the attached actually works.
This is fully backwards compatible with 1.2.x due to how hash_read
works.
Again, i'd badly like this change in 1.3.x.
(In 1.4, we probably want to not create the file at all. It turns out
not everything uses empty_props_p, however, so this actually introduces
an incompatibility)
For the curious, gcc's checkout has 30k files in it
each 4 byte file was taking 1 disk block of 4096 bytes.
30k * 4k = 120 meg
multiply by 2 (one props, one props-base), and you get 240 meg.
Whee.
This also stops useless I/O when a file has no properties set, as a
side-benefit.
I've applied this to the trunk in the meanwhile.
[[[
Don't write empty hashes to the props file, it just wastes space.
* subversion/libsvn_wc/props.c (svn_wc__save_prop_file): Check hash
count before writing to file.
(empty_props_p): finfo.size == 0 is empty too.
]]
Re: [PATCH]: Stop putting stuff in empty props files
Posted by Daniel Berlin <db...@dberlin.org>.
On Thu, 2005-10-20 at 22:56 +0400, Ivan Zhakov wrote:
> On 10/20/05, Daniel Berlin <db...@dberlin.org> wrote:
> > Actually, i think i can teach it to not create the props file if the
> > hash is empty, which works on all versions (they stat the path first,
> > and treat non-existence as emptyness).
> >
> > That works fine for my porpoises.
> I am going to try this tomorrow, but I consider it requires WC version
> bump and planned in wc-propcaching branch. Also I consider this could
> encourage problems in loggy code which copy/move props files.
>
Yes, you hit a couple log operations that don't check for existence.
If they called empty_props_p before doing things on the props, they'd
avoid this.
> --
> Ivan Zhakov
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: Stop putting stuff in empty props files
Posted by Ivan Zhakov <ch...@gmail.com>.
On 10/20/05, Daniel Berlin <db...@dberlin.org> wrote:
> Actually, i think i can teach it to not create the props file if the
> hash is empty, which works on all versions (they stat the path first,
> and treat non-existence as emptyness).
>
> That works fine for my porpoises.
I am going to try this tomorrow, but I consider it requires WC version
bump and planned in wc-propcaching branch. Also I consider this could
encourage problems in loggy code which copy/move props files.
--
Ivan Zhakov
Re: [PATCH]: Stop putting stuff in empty props files
Posted by Daniel Berlin <db...@dberlin.org>.
Actually, i think i can teach it to not create the props file if the
hash is empty, which works on all versions (they stat the path first,
and treat non-existence as emptyness).
That works fine for my porpoises.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org