On 9/19/06, Malcolm Rowe <malcolm-svn-dev@farside.org.uk> wrote:
> On Tue, Sep 19, 2006 at 12:38:02AM +0200, Erik Huelsmann wrote:
> > Well, the patch below addresses the problem in issue 2607.
> >
> > I've whipped this patch together this evening, so, I could use some
> > review (and comments about the idea), but, even at this
> > proof-of-concept level, the run-time of the test script seems to have
> > halved on my machine.
> >
>
> The general approach looks good.
Great.
> Are there any situations in which the fact that we're processing commit
> items out of order would make a difference?
Well, I sent the patch to zhakov and included a list of failing tests.
I thought that they were caused by out-of-order processing, but I've
added code to sort the adm's back into queue order before running
their logs.
Unfortunately, I'm getting the same list of failures...
> Some other observations:
>
> * The requirement to keep the arguments to svn_wc_queue_committed() alive
> until the call to svn_wc_process_committed_queue() is a little odd,
> but is probably better than copying each item into the queue's pool.
> We can always relax this requirement later.
Yea, we already keep all that data in memory during
svn_client_commit4(), so I did this to keep down the added size of the
constant factor to O(n)...
> * Why is remove_lock part of the commit queue item, but remove_changelist
> part of the commit operation? Wouldn't we want to the ability to
> remove changelists only for some of the commit items? (through the API;
> we wouldn't expose it through the CLI).
Moved.
> * You preallocate the queue to hold 40 items - how much memory does that
> actually work out to? What's the difference (speed or memory usage)
> between preallocating for 40 and just allocating one item?
No idea, but I'd say the added overhead should be negligeable w.r.t.
the amount of i/o in the post-commit processing.
> * It might be my mailer, but some of the changes to commit.c looks to
> be misindented.
That's your mailer :-)
> * The comments inside svn_wc_process_committed_queue() could make it
> clearer that we're writing to a log per queue item, then running the
> log per changed directory.
Ok. I tried to make it more clear, but given that there are still
failing tests, I don't expect to commit it yet anyway.
Revised patch attached. Still failing these tests:
FAIL: schedule_tests.py 10: status after add of deleted directory
FAIL: copy_tests.py 8: copy a tree and delete part of it before commit
FAIL: copy_tests.py 24: wc to wc copy with deleted=true items
FAIL: copy_tests.py 41: move a file twice within a moved dir
FAIL: copy_tests.py 42: move a file out of a moved dir
FAIL: copy_tests.py 43: move a dir twice within a moved dir
FAIL: copy_tests.py 44: move a dir out of a moved dir
FAIL: svnlook_tests.py 2: delete file in moved dir
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Sep 19 22:29:32 2006