Page MenuHomeFreeBSD

Limit git history searches in newvers.sh
ClosedPublic

Authored by gallatin on Fri, Jan 4, 4:23 PM.

Details

Summary

newvers.sh takes upwards of 4 seconds to complete on trees checked out from github, due to searching the entire history for non-existent git-svn metadata. Similarly, if one does not check out notes, we again search the entire history for notes. That makes newvers.sh very slow for many github users.

To fix this in a fair way, limit the history search to the last 10K commit: if you're more than 10K commits out of sync, then you've forked the project, and our SVN rev is no longer very important to you.

Due to how git implements --grep in conjunction with -n, it has been removed for performance reasons (git does not seem to limit its search to the -n limit in this case, and takes just as long as it did before).

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

gallatin created this revision.Fri, Jan 4, 4:23 PM
gallatin updated this revision to Diff 52553.Fri, Jan 4, 4:25 PM
emaste added inline comments.Fri, Jan 4, 4:30 PM
sys/conf/newvers.sh
255 ↗(On Diff #52552)

looks like the comment is cut off - it has forked the OS and the svn rev is not important?

emaste added inline comments.Fri, Jan 4, 4:34 PM
sys/conf/newvers.sh
255 ↗(On Diff #52552)

Oh, strange, the comment is indeed there - not sure what happened.

imp added a comment.Fri, Jan 4, 4:35 PM

10,000 is a bit arbitrary, but is a decent enough limit. 1k likely would do the trick, but maybe not always and wouldn't help the speed much at all. 100k likely would be measurably slower. I know that patch collections in monta vista linux for just the kernel ran into the thousands back in the day when I had to cope with their insanity, so maybe 10k isn't all that over-high...

sys/conf/newvers.sh
252 ↗(On Diff #52553)

kill the xxxx's please

emaste added a comment.Fri, Jan 4, 4:37 PM

I tried a few values locally and found:

LimitTime
10000.075
50000.180
100000.298
250000.645
500001.138
1000001.937
emaste accepted this revision.Fri, Jan 4, 4:38 PM

LGTM with @imp's comment about XXXs addressed.

This revision is now accepted and ready to land.Fri, Jan 4, 4:38 PM
gallatin updated this revision to Diff 52555.Fri, Jan 4, 5:02 PM

Remove XXX as per wlosh

This revision now requires review to proceed.Fri, Jan 4, 5:02 PM
gallatin marked an inline comment as done.Fri, Jan 4, 5:02 PM
emaste accepted this revision.Fri, Jan 4, 5:14 PM
This revision is now accepted and ready to land.Fri, Jan 4, 5:14 PM
This revision was automatically updated to reflect the committed changes.