[PATCH] buffer SVN _get_history for better performance

[csapuntz@…]

This leads to slow bitten build status pages and maybe other places in trac.

[csapuntz@…]

Diff to optimize _get_history in svn_fs.py

[csapuntz@…]

Second version of patch that tracks across path changes

[mgood]

Yeah, this buffered approach looks good, but I've got a couple of questions/comments.

• What is the purpose of the try/except block? I'd like to have a comment in the code explaining why the exception is being discarded, if that really is necessary.
• Instead of the changes made between the two versions of the patch it looks like you could just add this line after the "for item in history" loop:

  svn_path = _to_svn(item[0])
  

• The "if hasattr(...)" and nested function definitions should be pulled out of the loop since it's redundant to recheck and redefine these each iteration.

I can make those changes to the patch if you like, but I'd mainly like to know about the exceptions getting discarded before committing this.

[anonymous]

It looks like this problem may have already been fixed in 0.10.4dev. The _get_history is gone.

Replying to mgood:

* What is the purpose of the try/except block? I'd like to have a comment in the code explaining why the exception is being discarded, if that really is necessary.

If there are no revisions in the range, svn_repos_history2 throws an exception.

* Instead of the changes made between the two versions of the patch it looks like you could just add this line after the "for item in history" loop: {{{ #!python svn_path = _to_svn(item[0]) }}}

Sounds reasonable. Need to check that item is not None too?

* The "if hasattr(...)" and nested function definitions should be pulled out of the loop since it's redundant to recheck and redefine these each iteration.

My python isn't good so I don't feel confident attempting that. Also, I would guess the cost of that check and function definitions is dwarfed by the cost of calling svn_repos_history2.

I can make those changes to the patch if you like, but I'd mainly like to know about the exceptions getting discarded before committing this.

Also, I think subversion 1.2 and before throw SystemError? so the except should also filter that.

[mgood]

Yes, it appears that some with recent changes the code now iterates over the history instead of pulling all revisions for a path into memory which should solve the same issue this patch was meant to solve.

[cboos]

(see r4711)