Re: Caching text-size in the entries file

[Erik Huelsmann <eh...@gmail.com>]

On 9/25/06, Erik Huelsmann <eh...@gmail.com> wrote:
> I started working on a patch to record the size of the (unedited)
> working copy file in the entries file. Using 1 stat() call and this
> value, we can detect whether the working copy file has changed,
> without the need to detranslate.
>
> This change can get us 2 things:
>
>  - Gain some independence from mtimes: A file which changed in size
> clearly has changes, even if the mtimes stay the same
>  - Gain in performance in case there are translation-influencing
> properties in the game: currently, to establish 'changedness' of the
> wc file, libsvn_wc needs to detranslate it to compare to the
> text-base.
>
> These numbers is what I got from a (rudimentary implemented) patch for
> 'svn status' (in 2 runs):
>
> with keyword props
>
>           old      new
> run 1   1.8      0.1
> run 2    1.2      0.1
>
>
> without keyword props
>
>           old      new
> run 1    >0.1    ~0.1
> run 2    >0.1    ~0.1
>
>
> Given these numbers, I thought to implement the feature as follows:
>
> A new field (text-size) will be added to the svn_wc_entry_t structure.
> If this field contains a 0 (nul) value, the text-size cache is unused.
> Any other value means the working copy file size was originally that
> value.
>
> The value of the 'text-size' field will be kept up to date as follows:
>
> - all committed entries will have their text-size cached
> - all locally unmodified updated entries will have their text-size cached
> - all locally modified updated entries will have their text-size field
> set to 0 (nul)
> - moved and copied items have their entire entry copied (including the
> text-size value)
> - reverted items will have their text-size reset to the wc file size
> after revert
>
>
> This change will mean:
>
> - That we will be marking files which have edited keyword values as
> modified (we currently don't: we detranslate, making those changes
> invisible)
> - That we will be marking files with changed sizes as modified - even
> if the mtime hasn't changed - which we currently don't (they show up
> as unmodified)
> - We will gain 'svn status' speed with many changed files and
> svn:keywords (but no extra stat() calls for the case of where no files
> changed)
> - We gain some independence from mtimes

Given the lack of negative responses, I'm going to update the code for
current trunk and commit a change to this effect sometime next week.

bye,

Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Caching text-size in the entries file

[Erik Huelsmann <eh...@gmail.com>]

On 11/7/06, Michael Haggerty <mh...@alum.mit.edu> wrote:
> Erik Huelsmann wrote:
> > On 11/6/06, Erik Huelsmann <eh...@gmail.com> wrote:
> >> On 11/6/06, Peter Lundblad <pl...@google.com> wrote:
> >> > Erik Huelsmann writes:
> >> > > Ah, we already have a speed-vs-correctness tradeoff in status: we use
> >> > > mtimes instead of filecomparison or hash calculation. BTW: I do feel
> >> > Yeah, but I think we need to be careful when making the heuristic
> >> worse.
> >>
> >> Right, but what I hope to do is reduce the number of false negatives:
> >> I assert most files which are modified (but do not have their
> >> timestamp changed) won't have modified keywords or eols only. Rather:
> >> I think having modified eols-and-keyword-expansions-only is an edge
> >> case rather unlikely to happen. Whereas we have seen cases where
> >> timestamps were kept constant (making the changes undetected).
> >>
> >> Currently our algorithm doesn't know any false positives, but it has a
> >> chance for false negatives. I'd rather see that the other way around:
> >> false positives are correctable with an 'svn revert' or 'svn cleanup';
> >> false negatives don't have a cure.
>
> What if Erik's new algorithm is used to detect
> files-that-might-be-modified, then those files are double-checked using
> the more expensive algorithm?  I assume that in most use cases, at most
> a small percentage of files are changed when 'svn stat' is run.
> Therefore this should give almost as large a speed win without any
> downsides.

Well, that'll give us fewer false negatives, without the extra false
positives, but it will gain us no speedup: all files which are marked
'maybe-changed' need to be detranslated to test eol- and keywords-only
changes. My point is that these are sufficiently edge case not to
require full detranslation on status: 'normally' only content changes
will have occurred.

Hope that clarifies both the change and the consequences.

bye,


Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Caching text-size in the entries file

[Michael Haggerty <mh...@alum.mit.edu>]

Erik Huelsmann wrote:
> On 11/6/06, Erik Huelsmann <eh...@gmail.com> wrote:
>> On 11/6/06, Peter Lundblad <pl...@google.com> wrote:
>> > Erik Huelsmann writes:
>> > > Ah, we already have a speed-vs-correctness tradeoff in status: we use
>> > > mtimes instead of filecomparison or hash calculation. BTW: I do feel
>> > Yeah, but I think we need to be careful when making the heuristic
>> worse.
>>
>> Right, but what I hope to do is reduce the number of false negatives:
>> I assert most files which are modified (but do not have their
>> timestamp changed) won't have modified keywords or eols only. Rather:
>> I think having modified eols-and-keyword-expansions-only is an edge
>> case rather unlikely to happen. Whereas we have seen cases where
>> timestamps were kept constant (making the changes undetected).
>>
>> Currently our algorithm doesn't know any false positives, but it has a
>> chance for false negatives. I'd rather see that the other way around:
>> false positives are correctable with an 'svn revert' or 'svn cleanup';
>> false negatives don't have a cure.

What if Erik's new algorithm is used to detect
files-that-might-be-modified, then those files are double-checked using
the more expensive algorithm?  I assume that in most use cases, at most
a small percentage of files are changed when 'svn stat' is run.
Therefore this should give almost as large a speed win without any
downsides.

Michael

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Caching text-size in the entries file

[Erik Huelsmann <eh...@gmail.com>]

On 11/6/06, Erik Huelsmann <eh...@gmail.com> wrote:
> On 11/6/06, Peter Lundblad <pl...@google.com> wrote:
> > Erik Huelsmann writes:
> > > Ah, we already have a speed-vs-correctness tradeoff in status: we use
> > > mtimes instead of filecomparison or hash calculation. BTW: I do feel
> > Yeah, but I think we need to be careful when making the heuristic worse.
>
> Right, but what I hope to do is reduce the number of false negatives:
> I assert most files which are modified (but do not have their
> timestamp changed) won't have modified keywords or eols only. Rather:
> I think having modified eols-and-keyword-expansions-only is an edge
> case rather unlikely to happen. Whereas we have seen cases where
> timestamps were kept constant (making the changes undetected).
>
> Currently our algorithm doesn't know any false positives, but it has a
> chance for false negatives. I'd rather see that the other way around:
> false positives are correctable with an 'svn revert' or 'svn cleanup';
> false negatives don't have a cure.
>
> (I believe this is the semantic part.)
>
> > > that some speed improvements are worth more than others; this one is a
> > > factor 18 for translated files. I think that's a lot to gain. (Just
> > > yesterday had I a user complaining on IRC about our speed in the
> > > presence of keywords...)
> > >
> > Is this when most (all?) of the files are modified?
>
> Well, that was the testing scenario, but I can't imagine that the
> statistic wouldn't hold on a per-file basis :-)
>
> > > > Also, I think this has a compatibility impact. Currently, if you
> > > > call the status API and that returns the file as modified, then commit will
> > > > include it in the transaction. Users of the library might rely on this
> > > > semantic. If you have files that look modified before a commit, will
> > > > you still have those files look modified after the commit because they
> > > > were not considered modified by commit? Maybe the commit code could
> > > > detect and fix this?
> > >
> > > That's no problem. Ofcourse it could do that (post-commit!). But how
> > > about those files that are currently modified but look unmodified
> > > because of detranslation? We don't do anything about those, do we?
> > >
> > Do you mean modification that would be "normalized away" in the repository?
>
> Exactly.
>
> > > But, not all files reported as not-modified are not modified: we even
> > > have a test which creates an inconsistent new line in a file (by
> > > replacing CRLF with LF). Currently status says the file isn't
> > > modified, but if you have software which only tolerates CRLF, it
> > > *does* consider it modified.
> > >
> > I think here is where our views differ.  I have always assumed that
> > modified as reported by status means modified compared to the base
> > revision, not compared to what the text base looked like when it was
> > checked out. This means that a file that status says is modified will
> > look different if committed, compared to the base revision.
>
> I agree with the basic principle, but have a hard time accepting we'd
> have an algorithm which only knows false negatives, when we don't have
> a way of dealing with them, while false positives would be 'cureable',
> but we don't generate any of those.
>
> > > Why I think this change is important is: I think we should report
> > > 'Modified' as close as we can to the file actually having been
> > > modified (as viewed by the user).
> > >
> > So, is this a speed improvement or a semantic improvement?
>
> Well, I didn't mean for it to be a semantic change, but I guess we end
> up with one, yes. I meant for it to be a speed improvement.
>
> > If we implement
> > this change, we need to make sure we are consistent. Either a file
> > with mods inside keyword expanded parts is modified or not according
> > to status.
>
> Agreed.
>
> > This must not depend on whether the size is the same or
> > not. I would prefer that we discuss the semantic change separately
> > from the optimization. If we can agree that your proposed new
> > semantics are correct, then we can discuss performance improvements.
>
> Right, but what I'm seeing is that what status returns as modified
> truely needs to be committed, but what it returns as *unmodified* does
> not necessarily mean it wouldn'

sorry about that, I wasn't finished.

"not necessarily wouldn't be committed when passed as an argument to
'svn ci -m "" --force <target>". Isn't status there to help determine
what should and shouldn't (probably) be committed?


> > > Aren't we always going to confuse some users in some cases by doing
> > > translation anyway?
> > >
> > Probably:-) But we could at least be consistent.

To me, it looks like we have a gap (the heuristic vs full file
comparison) between what 'svn ci --force' should do and what 'svn st'
does. How can that be consistent?

bye,

Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Caching text-size in the entries file

[Erik Huelsmann <eh...@gmail.com>]

On 11/6/06, Peter Lundblad <pl...@google.com> wrote:
> Erik Huelsmann writes:
> > Ah, we already have a speed-vs-correctness tradeoff in status: we use
> > mtimes instead of filecomparison or hash calculation. BTW: I do feel
> Yeah, but I think we need to be careful when making the heuristic worse.

Right, but what I hope to do is reduce the number of false negatives:
I assert most files which are modified (but do not have their
timestamp changed) won't have modified keywords or eols only. Rather:
I think having modified eols-and-keyword-expansions-only is an edge
case rather unlikely to happen. Whereas we have seen cases where
timestamps were kept constant (making the changes undetected).

Currently our algorithm doesn't know any false positives, but it has a
chance for false negatives. I'd rather see that the other way around:
false positives are correctable with an 'svn revert' or 'svn cleanup';
false negatives don't have a cure.

(I believe this is the semantic part.)

> > that some speed improvements are worth more than others; this one is a
> > factor 18 for translated files. I think that's a lot to gain. (Just
> > yesterday had I a user complaining on IRC about our speed in the
> > presence of keywords...)
> >
> Is this when most (all?) of the files are modified?

Well, that was the testing scenario, but I can't imagine that the
statistic wouldn't hold on a per-file basis :-)

> > > Also, I think this has a compatibility impact. Currently, if you
> > > call the status API and that returns the file as modified, then commit will
> > > include it in the transaction. Users of the library might rely on this
> > > semantic. If you have files that look modified before a commit, will
> > > you still have those files look modified after the commit because they
> > > were not considered modified by commit? Maybe the commit code could
> > > detect and fix this?
> >
> > That's no problem. Ofcourse it could do that (post-commit!). But how
> > about those files that are currently modified but look unmodified
> > because of detranslation? We don't do anything about those, do we?
> >
> Do you mean modification that would be "normalized away" in the repository?

Exactly.

> > But, not all files reported as not-modified are not modified: we even
> > have a test which creates an inconsistent new line in a file (by
> > replacing CRLF with LF). Currently status says the file isn't
> > modified, but if you have software which only tolerates CRLF, it
> > *does* consider it modified.
> >
> I think here is where our views differ.  I have always assumed that
> modified as reported by status means modified compared to the base
> revision, not compared to what the text base looked like when it was
> checked out. This means that a file that status says is modified will
> look different if committed, compared to the base revision.

I agree with the basic principle, but have a hard time accepting we'd
have an algorithm which only knows false negatives, when we don't have
a way of dealing with them, while false positives would be 'cureable',
but we don't generate any of those.

> > Why I think this change is important is: I think we should report
> > 'Modified' as close as we can to the file actually having been
> > modified (as viewed by the user).
> >
> So, is this a speed improvement or a semantic improvement?

Well, I didn't mean for it to be a semantic change, but I guess we end
up with one, yes. I meant for it to be a speed improvement.

> If we implement
> this change, we need to make sure we are consistent. Either a file
> with mods inside keyword expanded parts is modified or not according
> to status.

Agreed.

> This must not depend on whether the size is the same or
> not. I would prefer that we discuss the semantic change separately
> from the optimization. If we can agree that your proposed new
> semantics are correct, then we can discuss performance improvements.

Right, but what I'm seeing is that what status returns as modified
truely needs to be committed, but what it returns as *unmodified* does
not necessarily mean it wouldn'

> > Aren't we always going to confuse some users in some cases by doing
> > translation anyway?
> >
> Probably:-) But we could at least be consistent.
>
> Regards,
> //Peter
> \
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Caching text-size in the entries file

[Peter Lundblad <pl...@google.com>]

Erik Huelsmann writes:
> Ah, we already have a speed-vs-correctness tradeoff in status: we use
> mtimes instead of filecomparison or hash calculation. BTW: I do feel

Yeah, but I think we need to be careful when making the heuristic worse.

> that some speed improvements are worth more than others; this one is a
> factor 18 for translated files. I think that's a lot to gain. (Just
> yesterday had I a user complaining on IRC about our speed in the
> presence of keywords...)
> 
Is this when most (all?) of the files are modified?


> > Also, I think this has a compatibility impact. Currently, if you
> > call the status API and that returns the file as modified, then commit will
> > include it in the transaction. Users of the library might rely on this
> > semantic. If you have files that look modified before a commit, will
> > you still have those files look modified after the commit because they
> > were not considered modified by commit? Maybe the commit code could
> > detect and fix this?
> 
> That's no problem. Ofcourse it could do that (post-commit!). But how
> about those files that are currently modified but look unmodified
> because of detranslation? We don't do anything about those, do we?
> 
Do you mean modification that would be "normalized away" in the repository?


> But, not all files reported as not-modified are not modified: we even
> have a test which creates an inconsistent new line in a file (by
> replacing CRLF with LF). Currently status says the file isn't
> modified, but if you have software which only tolerates CRLF, it
> *does* consider it modified.
> 
I think here is where our views differ.  I have always assumed that
modified as reported by status means modified compared to the base
revision, not compared to what the text base looked like when it was
checked out. This means that a file that status says is modified will
look different if committed, compared to the base revision.

> Why I think this change is important is: I think we should report
> 'Modified' as close as we can to the file actually having been
> modified (as viewed by the user).
> 
So, is this a speed improvement or a semantic improvement? If we implement
this change, we need to make sure we are consistent. Either a file
with mods inside keyword expanded parts is modified or not according
to status. This must not depend on whether the size is the same or
not. I would prefer that we discuss the semantic change separately
from the optimization. If we can agree that your proposed new
semantics are correct, then we can discuss performance improvements.

> Aren't we always going to confuse some users in some cases by doing
> translation anyway?
> 
Probably:-) But we could at least be consistent.

Regards,
//Peter
\

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Caching text-size in the entries file

[Erik Huelsmann <eh...@gmail.com>]

On 11/6/06, Peter Lundblad <pl...@google.com> wrote:
> Erik Huelsmann writes:
> > > This change will mean:
> > >
> > > - That we will be marking files which have edited keyword values as
> > > modified (we currently don't: we detranslate, making those changes
> > > invisible)
> > > - That we will be marking files with changed sizes as modified - even
> > > if the mtime hasn't changed - which we currently don't (they show up
> > > as unmodified)
> > > - We will gain 'svn status' speed with many changed files and
> > > svn:keywords (but no extra stat() calls for the case of where no files
> > > changed)
> > > - We gain some independence from mtimes
> >
> > Given the lack of negative responses, I'm going to update the code for
> > current trunk and commit a change to this effect sometime next week.
> >
>
> Sorry for not paying attention earlier, but I am not sure I like this
> speed vs. correctness tradeoff. I think it is confusing to users to show
> files as modified that won't be modified in a commit.

Ah, we already have a speed-vs-correctness tradeoff in status: we use
mtimes instead of filecomparison or hash calculation. BTW: I do feel
that some speed improvements are worth more than others; this one is a
factor 18 for translated files. I think that's a lot to gain. (Just
yesterday had I a user complaining on IRC about our speed in the
presence of keywords...)

> Also, I think this has a compatibility impact. Currently, if you
> call the status API and that returns the file as modified, then commit will
> include it in the transaction. Users of the library might rely on this
> semantic. If you have files that look modified before a commit, will
> you still have those files look modified after the commit because they
> were not considered modified by commit? Maybe the commit code could
> detect and fix this?

That's no problem. Ofcourse it could do that (post-commit!). But how
about those files that are currently modified but look unmodified
because of detranslation? We don't do anything about those, do we?

But, not all files reported as not-modified are not modified: we even
have a test which creates an inconsistent new line in a file (by
replacing CRLF with LF). Currently status says the file isn't
modified, but if you have software which only tolerates CRLF, it
*does* consider it modified.

Why I think this change is important is: I think we should report
'Modified' as close as we can to the file actually having been
modified (as viewed by the user).

Aren't we always going to confuse some users in some cases by doing
translation anyway?

bye,

Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Caching text-size in the entries file

[Peter Lundblad <pl...@google.com>]

Erik Huelsmann writes:
> > This change will mean:
> >
> > - That we will be marking files which have edited keyword values as
> > modified (we currently don't: we detranslate, making those changes
> > invisible)
> > - That we will be marking files with changed sizes as modified - even
> > if the mtime hasn't changed - which we currently don't (they show up
> > as unmodified)
> > - We will gain 'svn status' speed with many changed files and
> > svn:keywords (but no extra stat() calls for the case of where no files
> > changed)
> > - We gain some independence from mtimes
> 
> Given the lack of negative responses, I'm going to update the code for
> current trunk and commit a change to this effect sometime next week.
> 

Sorry for not paying attention earlier, but I am not sure I like this
speed vs. correctness tradeoff. I think it is confusing to users to show
files as modified that won't be modified in a commit.

Also, I think this has a compatibility impact. Currently, if you
call the status API and that returns the file as modified, then commit will
include it in the transaction. Users of the library might rely on this
semantic. If you have files that look modified before a commit, will
you still have those files look modified after the commit because they
were not considered modified by commit? Maybe the commit code could
detect and fix this?

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org