You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ben Collins-Sussman <su...@red-bean.com> on 2009/01/08 15:25:20 UTC

philosophical questions about mailer.py

Howdy folks,

We've been using mailer.py over at Google Code for years now (as if
the emails we generate weren't obvious proof of that!)

Recently, one of our engineers was scratching his head, wondering why
commits that modify thousands of files (e.g. an initial import) were
taking so incredibly long to generate emails for.  It turns out that
mailer.py implements a "ChangeCollector" editor, and then asks
svn_repos_replay() to replay a single revision into it.  This seems
like a really gigantic amount of work when dealing with large commits:

  * brute-force compare revisions N-1 and N
  * rediscover and store every changed path in a huge list
  * iterate over the list, fetching files and generating diffs

For the first step, why aren't we just calling svn_repos_get_logs() to
instantly fetched the changed-paths list?

On the one hand, I understand that replay() is "perfectly correct", in
that it avoids some very rare edge-case bugs:  (IIRC, get_logs() can't
tell the difference between a file that was copied, and one which was
copied and edited in the same revision.)  Clearly, the 100% accuracy
of replay() is necessary for a tool like svnsync.  But is this
accuracy really necessary for generating commit emails?  Wouldn't the
commit emails generated via get_logs() and replay() look the same,
even in these edge-cases?   We're paying a really heavy performance
price for replay() -- particularly for large commits -- and I wonder
if there's any benefit at all.   My gut is to make mailer.py go back
to using get_logs() (or at least create an option for it).

Re: philosophical questions about mailer.py

Posted by Ben Collins-Sussman <su...@red-bean.com>.
On Thu, Jan 8, 2009 at 10:20 AM, Ben Collins-Sussman
<su...@red-bean.com> wrote:
> On Thu, Jan 8, 2009 at 10:09 AM, C. Michael Pilato <cm...@collab.net> wrote:
>
>> Oh, something else I thought of:  the translation between C and Python has,
>> in my experience, proven to be very costly.  And svn_repos_replay() driving
>> a ChangeCollector in this situation crosses that translation boundary at
>> least 850 times.  :-)
>>
>
> Hm, yeah, I considered that too, but our local SWIG expert here says
> that it's not *that* costly.  Even crossing that swig boundary 1000
> times shouldn't be more than a couple of lost seconds.
>
> I guess another possibility is that the "extra" work replay() is doing
> on each changed path is somehow horrendously slow on our fs_bigtable
> implementation.  I could test that, I suppose, by doing an svnsync of
> this revision from the googlecode repository to a local FSFS
> repository (the local write/commit time would be pretty tiny.)
>

Hm, nope.  svnsync'ing that 850-change revision from googlecode to a
local FSFS repository took only 15 seconds.

Re: philosophical questions about mailer.py

Posted by "C. Michael Pilato" <cm...@collab.net>.
Ben Collins-Sussman wrote:
> On Thu, Jan 8, 2009 at 10:09 AM, C. Michael Pilato <cm...@collab.net> wrote:
> 
>> Oh, something else I thought of:  the translation between C and Python has,
>> in my experience, proven to be very costly.  And svn_repos_replay() driving
>> a ChangeCollector in this situation crosses that translation boundary at
>> least 850 times.  :-)
>>
> 
> Hm, yeah, I considered that too, but our local SWIG expert here says
> that it's not *that* costly.  Even crossing that swig boundary 1000
> times shouldn't be more than a couple of lost seconds.
> 
> I guess another possibility is that the "extra" work replay() is doing
> on each changed path is somehow horrendously slow on our fs_bigtable
> implementation.  I could test that, I suppose, by doing an svnsync of
> this revision from the googlecode repository to a local FSFS
> repository (the local write/commit time would be pretty tiny.)

Man, I don't know.  I don't carry the title "SWIG expert" or anything, but
do you remember when I was tweaking cvs2svn so that rather than running
'svnadmin load' as an external tool, it was using the svn_repos_load_fs()
APIs via the Python bindings?  I was seeing something like 2x timings (which
was why I abandoned the concept).

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1011930

Re: philosophical questions about mailer.py

Posted by Ben Collins-Sussman <su...@red-bean.com>.
On Thu, Jan 8, 2009 at 10:09 AM, C. Michael Pilato <cm...@collab.net> wrote:

> Oh, something else I thought of:  the translation between C and Python has,
> in my experience, proven to be very costly.  And svn_repos_replay() driving
> a ChangeCollector in this situation crosses that translation boundary at
> least 850 times.  :-)
>

Hm, yeah, I considered that too, but our local SWIG expert here says
that it's not *that* costly.  Even crossing that swig boundary 1000
times shouldn't be more than a couple of lost seconds.

I guess another possibility is that the "extra" work replay() is doing
on each changed path is somehow horrendously slow on our fs_bigtable
implementation.  I could test that, I suppose, by doing an svnsync of
this revision from the googlecode repository to a local FSFS
repository (the local write/commit time would be pretty tiny.)

RE: philosophical questions about mailer.py

Posted by Bert Huijben <rh...@sharpsvn.net>.
> -----Original Message-----
> From: C. Michael Pilato [mailto:cmpilato@collab.net]
> Sent: donderdag 8 januari 2009 17:10
> To: Ben Collins-Sussman
> Cc: dev@subversion.tigris.org
> Subject: Re: philosophical questions about mailer.py
> 
> Ben Collins-Sussman wrote:
> > On Thu, Jan 8, 2009 at 9:48 AM, C. Michael Pilato
> <cm...@collab.net> wrote:
> >
> >> Here's where I think the benefit to mailer.py will come (and I say
> this as a
> >> way of saying, "You may be onto something here, so keep
> investigating"):
> >> mailer.py doesn't need to present it's results in a tree structure.
> So all
> >> the work that _replay() does to take a flat list of paths and
> regenerate the
> >> tree structure is almost immediately discarded by the consumer.
> That's a
> >> lot of wasted processing.
> >
> > We're seeing some insane numbers here.  This command takes only 5
> > seconds to print out all 850 changed-paths:
> >
> >   $ svn log -v http://phpcommunityorg.googlecode.com/svn -r31
> >
> > And yet when we run mailer.py on this revision, it takes 68 seconds
> to
> > finish, and *60 seconds* of that is just in the replay() routine.
> > Something is very wrong.   We'll have to investigate a bit more.  :-)
> 
> Oh, something else I thought of:  the translation between C and Python
> has,
> in my experience, proven to be very costly.  And svn_repos_replay()
> driving
> a ChangeCollector in this situation crosses that translation boundary
> at
> least 850 times.  :-)

For all changed files in the editor drive: open_file/add_file, at least one
action (property change or content change), and then a close_file. And on
top of that some extra calls for the directory levels.. So I would guess at
least 2500 translations between C and Python for this specific example.

(I just wrapped the delta editor in SharpSvn)

	Bert

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1012067

Re: philosophical questions about mailer.py

Posted by "C. Michael Pilato" <cm...@collab.net>.
Ben Collins-Sussman wrote:
> On Thu, Jan 8, 2009 at 9:48 AM, C. Michael Pilato <cm...@collab.net> wrote:
> 
>> Here's where I think the benefit to mailer.py will come (and I say this as a
>> way of saying, "You may be onto something here, so keep investigating"):
>> mailer.py doesn't need to present it's results in a tree structure.  So all
>> the work that _replay() does to take a flat list of paths and regenerate the
>> tree structure is almost immediately discarded by the consumer.  That's a
>> lot of wasted processing.
> 
> We're seeing some insane numbers here.  This command takes only 5
> seconds to print out all 850 changed-paths:
> 
>   $ svn log -v http://phpcommunityorg.googlecode.com/svn -r31
> 
> And yet when we run mailer.py on this revision, it takes 68 seconds to
> finish, and *60 seconds* of that is just in the replay() routine.
> Something is very wrong.   We'll have to investigate a bit more.  :-)

Oh, something else I thought of:  the translation between C and Python has,
in my experience, proven to be very costly.  And svn_repos_replay() driving
a ChangeCollector in this situation crosses that translation boundary at
least 850 times.  :-)

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1011901

Re: philosophical questions about mailer.py

Posted by Ben Collins-Sussman <su...@red-bean.com>.
On Thu, Jan 8, 2009 at 9:48 AM, C. Michael Pilato <cm...@collab.net> wrote:

> Here's where I think the benefit to mailer.py will come (and I say this as a
> way of saying, "You may be onto something here, so keep investigating"):
> mailer.py doesn't need to present it's results in a tree structure.  So all
> the work that _replay() does to take a flat list of paths and regenerate the
> tree structure is almost immediately discarded by the consumer.  That's a
> lot of wasted processing.

We're seeing some insane numbers here.  This command takes only 5
seconds to print out all 850 changed-paths:

  $ svn log -v http://phpcommunityorg.googlecode.com/svn -r31

And yet when we run mailer.py on this revision, it takes 68 seconds to
finish, and *60 seconds* of that is just in the replay() routine.
Something is very wrong.   We'll have to investigate a bit more.  :-)

Re: philosophical questions about mailer.py

Posted by "C. Michael Pilato" <cm...@collab.net>.
Ben Collins-Sussman wrote:
>> Hm.  I think you don't understand svn_repos_replay().  It isn't a
>> brute-force comparison.  It fetches the list of changed paths and then,
>> using path-math, drives an editor to describe the changes.  (Read the big
>> comment atop libsvn_repos/replay.c)
> 
> Hm, okay, then we need to investigate why get_logs() is so quick
> compared to replay() on big imports -- something is fishy, for sure.
> 
>> So, yes, you can make mailer.py just fetch the changed paths.  And then you
>> can make it do all the work of tracking locations changes that occur with
>> copies and moves (so you can accurately generate diffs).  But now you've
>> practically implemented svn_repos_replay + the ChangeCollector.  :-)
> 
> Doh.

Well, _replay() is necessarily slower than just fetching the list of changed
paths.  (That's a guarantee, because a fetch of the list of changed paths is
the first thing that _replay() does!)  And the measured cost of _replay()
will include the cost of any editor it drives, right?

Here's where I think the benefit to mailer.py will come (and I say this as a
way of saying, "You may be onto something here, so keep investigating"):
mailer.py doesn't need to present it's results in a tree structure.  So all
the work that _replay() does to take a flat list of paths and regenerate the
tree structure is almost immediately discarded by the consumer.  That's a
lot of wasted processing.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1011870

Re: philosophical questions about mailer.py

Posted by Ben Collins-Sussman <su...@red-bean.com>.
On Thu, Jan 8, 2009 at 9:32 AM, C. Michael Pilato <cm...@collab.net> wrote:
> Ben Collins-Sussman wrote:
>> Howdy folks,
>>
>> We've been using mailer.py over at Google Code for years now (as if
>> the emails we generate weren't obvious proof of that!)
>>
>> Recently, one of our engineers was scratching his head, wondering why
>> commits that modify thousands of files (e.g. an initial import) were
>> taking so incredibly long to generate emails for.
>
> I've read this a few times now, and I'm left scratching my head.  Here's
> what I see:
>
>   "We're trying to figure out why an operation that does a ton of
>    stuff takes a while to describe."
>
> Come again?  What part of this was unexpected?

We expected the diff-generation on thousands files to take a long
time, sure... but we are seeing just *replay* take a ridiculously long
time to run (on the order of 10's of minutes), whereas a direct call
to get_logs() only takes maybe 20 seconds.   So I assumed that replay
was doing brute-force tree-comparisons!


> Hm.  I think you don't understand svn_repos_replay().  It isn't a
> brute-force comparison.  It fetches the list of changed paths and then,
> using path-math, drives an editor to describe the changes.  (Read the big
> comment atop libsvn_repos/replay.c)

Hm, okay, then we need to investigate why get_logs() is so quick
compared to replay() on big imports -- something is fishy, for sure.


>
> So, yes, you can make mailer.py just fetch the changed paths.  And then you
> can make it do all the work of tracking locations changes that occur with
> copies and moves (so you can accurately generate diffs).  But now you've
> practically implemented svn_repos_replay + the ChangeCollector.  :-)

Doh.

Re: philosophical questions about mailer.py

Posted by "C. Michael Pilato" <cm...@collab.net>.
Ben Collins-Sussman wrote:
> Howdy folks,
> 
> We've been using mailer.py over at Google Code for years now (as if
> the emails we generate weren't obvious proof of that!)
> 
> Recently, one of our engineers was scratching his head, wondering why
> commits that modify thousands of files (e.g. an initial import) were
> taking so incredibly long to generate emails for.

I've read this a few times now, and I'm left scratching my head.  Here's
what I see:

   "We're trying to figure out why an operation that does a ton of
    stuff takes a while to describe."

Come again?  What part of this was unexpected?

> It turns out that
> mailer.py implements a "ChangeCollector" editor, and then asks
> svn_repos_replay() to replay a single revision into it.  This seems
> like a really gigantic amount of work when dealing with large commits:
> 
>   * brute-force compare revisions N-1 and N
>   * rediscover and store every changed path in a huge list
>   * iterate over the list, fetching files and generating diffs
> 
> For the first step, why aren't we just calling svn_repos_get_logs() to
> instantly fetched the changed-paths list?
>
> On the one hand, I understand that replay() is "perfectly correct", in
> that it avoids some very rare edge-case bugs:  (IIRC, get_logs() can't
> tell the difference between a file that was copied, and one which was
> copied and edited in the same revision.)  Clearly, the 100% accuracy
> of replay() is necessary for a tool like svnsync.  But is this
> accuracy really necessary for generating commit emails?  Wouldn't the
> commit emails generated via get_logs() and replay() look the same,
> even in these edge-cases?   We're paying a really heavy performance
> price for replay() -- particularly for large commits -- and I wonder
> if there's any benefit at all.   My gut is to make mailer.py go back
> to using get_logs() (or at least create an option for it).

Hm.  I think you don't understand svn_repos_replay().  It isn't a
brute-force comparison.  It fetches the list of changed paths and then,
using path-math, drives an editor to describe the changes.  (Read the big
comment atop libsvn_repos/replay.c)

So, yes, you can make mailer.py just fetch the changed paths.  And then you
can make it do all the work of tracking locations changes that occur with
copies and moves (so you can accurately generate diffs).  But now you've
practically implemented svn_repos_replay + the ChangeCollector.  :-)



-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1011846