Mark,
I finally got around to completing this change, attached is a
unified diff (I attached it because the last patch was corrupted by
outlook) against cvs 1.11.21.
This patch is only for the stable release, once it's been looked
over and approved, I'll do the feature release, I just didn't want to
get it all done and then find out the approach was flawed.
I've included ChangeLog and sanity.sh changes, I believe the
sanity.sh tests are sufficiently paranoid, they did point out failures
with the previous patch.
The previous patch improved my repo's branch time from 115
minutes to 8 minutes, this patch only gets it down to 12 minutes, but
since it's correct, I won't worry about the 4 "extra" minutes! ;>
Let me know if there's anything that I missed, or if I can help
in any way to get this patch accepted.
Thanks,
--
Kelly F. Hickel
Senior Software Architect
MQSoftware, Inc
952.345.8677
[EMAIL PROTECTED]
> -----Original Message-----
> From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Mark D.
> Baushke
> Sent: Wednesday, May 10, 2006 10:33 AM
> To: Kelly F. Hickel
> Cc: [email protected]
> Subject: Re: Proposed branch tag performance patch for feature and
> stable releases
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi Kelly,
>
> Point #1 of this has been communicated privately, but it occurs to me
> that the bug-cvs list may be interested in knowing what is happening.
>
> There are some problems with the proposed patch:
>
> 1) It is finding the the highest revision across all branches with
> the
> right number of dots, not just the branches rooted at the same
> revision. So, if you have a number of branches off 1.1 --
> 1.1.0.2,
> 1.1.0.4, and 1.1.0.6, for example -- and then create a new branch
> off of 1.2 you will get 1.2.0.8 instead of 1.2.0.2 as you should.
> It is also checking all tags with the right number of dots, not
> just magic branch tags.
>
> 2) For the FEATURE branch (1.12.x), there is an added problem in
> that
> the file may be using a mixture of the CVS_LOCAL_BRANCH_NUM
> feature
> along with regular branches. In this case, it is a good bet that
> the user is using CVSup to make sure that revisions that are
> above
> the CVS_LOCAL_BRANCH_NUM starting number will NOT be propagated
> to
> mirrors of the repository and are thus not generally available.
>
> So, if you have a number of global branches off 1.1 -- 1.1.0.2,
> 1.1.0.4, 1.1.0.6 as well as a few local branches 1.1.0.1000 and
> 1.1.0.1002, then the next global branch created off of 1.1 will
> incorrectly be set to 1.1.0.1004 instead of 1.1.0.8 as expected.
>
> Of course, this may not be that big a deal as in the normal
> course
> of things new global branches are created on the master
> repository
> and CVSup'd to the mirrors and the global will typically not have
> a
> good reason to create a CVS_LOCAL_BRANCH_NUM, however, it has
> happened that master repositories become dead and a new master is
> elected from one of the mirrors which could lead to the above
> problem.
>
> 3) The patch provided did pass our sanity.sh regression tests and
> should not have. This means that we likely need to have a more
> rigorous set of tests that the correct revisions are being
> created
> by the cvs tag code. So, the proposed patch will probably also
> need
> to include some sanity.sh tests to exercise it.
>
> Thank you for your consideration of this problem.
>
> -- Mark
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.3 (FreeBSD)
>
> iD8DBQFEYgevCg7APGsDnFERAomSAKDMWgJWcmOByJRv0h2J1kZrW3fGvwCfZjkR
> jtI9cRWSisG6ORRlK0CVQvE=
> =jvvC
> -----END PGP SIGNATURE-----
diff -ru cvs-1.11.21_orig/src/ChangeLog cvs-1.11.21/src/ChangeLog
--- cvs-1.11.21_orig/src/ChangeLog 2005-09-26 09:31:33.000000000 -0500
+++ cvs-1.11.21/src/ChangeLog 2007-01-03 13:54:28.000000000 -0600
@@ -1,3 +1,13 @@
+2007-01-03 Kelly F. Hickel <[EMAIL PROTECTED]>
+
+ * rcs.c (findnextmagicrev_proc): New function to find the
+ all rev numbers with the required number of dots and matching
+ initial substring.
+ (findnextmagicrev): New function to walk the symbolic tag list
+ calling findnextmagicrev_proc() to get a list of used rev numbers
+ and choose the highest suitable one.
+ (RCS_magicrev): Use findnextmagicrev().
+
2005-09-26 Conrad T. Pino <[EMAIL PROTECTED]>
* rcs.c: Use "#ifdef HAVE_FSYNC" just like every where else.
diff -ru cvs-1.11.21_orig/src/rcs.c cvs-1.11.21/src/rcs.c
--- cvs-1.11.21_orig/src/rcs.c 2005-09-26 09:31:36.000000000 -0500
+++ cvs-1.11.21/src/rcs.c 2007-01-03 13:56:27.000000000 -0600
@@ -133,6 +133,8 @@
static FILE *rcs_internal_lockfile PROTO ((char *));
static void rcs_internal_unlockfile PROTO ((FILE *, char *));
static char *rcs_lockfilename PROTO ((const char *));
+static int findnextmagicrev PROTO ((RCSNode *rcs, char *rev, int default_rv));
+static int findnextmagicrev_proc PROTO((Node *p, void *closure));
/* The RCS file reading functions are called a lot, and they do some
string comparisons. This macro speeds things up a bit by skipping
@@ -2549,8 +2551,13 @@
xrev = xmalloc (strlen (rev) + 14); /* enough for .0.number */
check_rev = xrev;
+ /* prime the pump by finding the next unused magic rev,
+ * if none are found, it should return 2.
+ */
+ rev_num = findnextmagicrev (rcs, rev, 2);
+
/* only look at even numbered branches */
- for (rev_num = 2; ; rev_num += 2)
+ for (; ; rev_num += 2)
{
/* see if the physical branch exists */
(void) sprintf (xrev, "%s.%d", rev, rev_num);
@@ -8818,3 +8825,181 @@
}
return label;
}
+
+/* closure structure to communicate through walklist */
+struct findnextmagicrev_info {
+ char *target_rev;
+ int target_rev_dots;
+ int target_rev_len;
+ List *rev_list;
+};
+
+/* sort a list assuming that the data members are integers */
+static int
+findnextmagicrev_sortfunc(p, q)
+ const Node *p;
+ const Node *q;
+{
+ /* We want the highest value sorted first, to make it easy to find */
+ return ((int)q->data - (int)p->data);
+}
+
+/* free a list node where the data member is an integer */
+static void
+findnextmagicrev_delproc(Node *p)
+{
+ /* Do nothing, it's not a pointer */
+ p->data = 0;
+}
+
+/*
+ * Go through the symbolic tag list, find the next unused magic
+ * branch revision.
+ *
+ * Returns defaultrv if it can't figure anything out, then the caller
+ * will end up doing a linear search.
+ */
+static int
+findnextmagicrev (rcs, rev, defaultrv)
+ RCSNode *rcs;
+ char *rev;
+ int defaultrv;
+{
+ int rv = defaultrv;
+ struct findnextmagicrev_info info;
+
+ /* Tell the walklist proc how many dots we're looking for,
+ * which is the number of dots in the existing rev, plus
+ * 2. one for RCS_MAGIC_BRANCH and one for the new rev number.
+ */
+ info.target_rev = rev;
+ info.target_rev_dots = numdots (rev) + 2;
+ info.target_rev_len = strlen(rev);
+ info.rev_list = getlist();
+
+ /* walk the symbols list to find the highest revision. */
+ (void) walklist (RCS_symbols (rcs), findnextmagicrev_proc, &info);
+
+ if(!list_isempty(info.rev_list))
+ {
+ int i=0;
+ /* sort the list that came back */
+ sortlist(info.rev_list, findnextmagicrev_sortfunc);
+
+ rv = (int)info.rev_list->list->next->data; /* Get the highest numeric
value */
+ /* adjust to next even number if we found something */
+ if (rv != defaultrv)
+ {
+ if ((rv % 2) != 0)
+ rv++;
+ else
+ rv += 2;
+ }
+ }
+
+ dellist(&(info.rev_list));
+ return rv;
+}
+
+/*
+ * walklist proc to find all "interesting" revs,
+ * and put them in a list for the caller to look through.
+ * Interesting is defined as having numdots+2 and same initial
+ * substring (up to the dot of numdots+1), or the second to last
+ * term indicates this is a magic branch.
+ */
+static int
+findnextmagicrev_proc (p, closure)
+ Node *p;
+ void *closure;
+{
+ struct findnextmagicrev_info *pinfo = (struct findnextmagicrev_info
*)closure;
+ int sym_dots = numdots(p->data);
+
+ /*
+ * Quick first level screen, if the number of dots isn't right, then we
+ * don't care about this revision.
+ */
+ if(sym_dots == pinfo->target_rev_dots)
+ {
+ /*
+ * We can't rely on being able to find the magic branch tag to find all
+ * existing branches, as that may have been deleted. So, here we're
looking
+ * for revisions where the initial substring matches our target, and
+ * that have the same number of dots that revisions in the new
+ * branch will have in this case we record atoi of the second to last
+ * term in the list.
+ * We're also looking at revisions with RCS_MAGIC_BRANCH as the second to
+ * last term, for those, we record atoi of the last term in the list.
+ * Example:
+ * We're about to create a branch from 1.1, if we see revisions like
+ * 1.1.2.1, or 1.1.6.1, then we know we can't use either 2 or 6 as the
new
+ * rev term, even if we don't see the symbols 1.1.0.2 and 1.1.0.6.
+ * If we see revisions like 1.1.0.8 or 1.1.0.12, we know that we can't
use
+ * 8 or 12.
+ *
+ * Simply put, for revistions that have the same initial substring as our
+ * target, if the second to last term is RCS_MAGIC_BRANCH, take atoi of
the
+ * last term and put it in the list, if not, take atoi of the
+ * second to last term, put it in the list.
+ */
+
+ /*
+ * Second screen, make sure that target_rev is an initial substring
+ * of the rev we're considering. This means doing a strncmp, then if
+ * it's the same, making sure that the one under consideration has a
+ * dot after the target rev.
+ *
+ * Check the dot first, since that's faster.
+ */
+ char *ver_str = (char*)(p->data);
+ if((ver_str[pinfo->target_rev_len] == '.') &&
+ (strncmp(pinfo->target_rev, p->data, pinfo->target_rev_len) == 0)) {
+ char *plast_dot = 0;
+ char *psecond_to_last_dot = 0;
+ int i = strlen(ver_str);
+
+ /* Get pointers in this revision to the last and second to last dots */
+ while((i>0) && ((plast_dot == 0) || (psecond_to_last_dot == 0)))
+ {
+ if(ver_str[i] == '.')
+ {
+ if(plast_dot == 0)
+ {
+ plast_dot = &(ver_str[i]);
+ }
+ else
+ {
+ psecond_to_last_dot = &(ver_str[i]);
+ }
+ }
+ i--;
+ }
+ if((plast_dot != 0) && (psecond_to_last_dot != 0))
+ {
+ int term_to_add = 0;
+ int second_to_last_term = atoi (psecond_to_last_dot+1);
+ if(second_to_last_term == RCS_MAGIC_BRANCH)
+ {
+ /* We want to put the value of the last term into the list */
+ term_to_add = atoi (plast_dot+1);
+ }
+ else
+ {
+ /* We want to put the value of the second to last term into the
list */
+ term_to_add = atoi (psecond_to_last_dot+1);
+ }
+ if(term_to_add != 0)
+ {
+ Node *n = getnode ();
+ n->type = VARIABLE;
+ n->key = 0;
+ n->data = (void*)term_to_add;
+ n->delproc = findnextmagicrev_delproc;
+ addnode (pinfo->rev_list, n); /* dupe insert won't fail because
we aren't using a hash */
+ }
+ }
+ }
+ }
+ return 1;
+}
diff -ru cvs-1.11.21_orig/src/sanity.sh cvs-1.11.21/src/sanity.sh
--- cvs-1.11.21_orig/src/sanity.sh 2005-09-22 12:09:18.000000000 -0500
+++ cvs-1.11.21/src/sanity.sh 2007-01-03 08:02:54.000000000 -0600
@@ -6975,6 +6975,187 @@
rm -r trunk b1a b1b
;;
+ branches3)
+ # test new faster branching code to make sure it doesn't reuse a
previously
+ # used magic branch tag.
+ # Set up a repo, files, etc.
+ mkdir ${CVSROOT_DIRNAME}/first-dir
+ dotest branches3-1 "${testcvs} -q co first-dir" ''
+ cd first-dir
+ echo 1:ancest >file1
+ echo 2:ancest >file2
+ dotest branches3-2 "${testcvs} add file1 file2" \
+"${PROG}"' add: scheduling file `file1'\'' for addition
+'"${PROG}"' add: scheduling file `file2'\'' for addition
+'"${PROG}"' add: use .'"${PROG}"' commit. to add these files permanently'
+ dotest_lit branches3-3a "${testcvs} -q ci -m add-it" <<HERE
+RCS file: ${CVSROOT_DIRNAME}/first-dir/file1,v
+done
+Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+initial revision: 1.1
+done
+RCS file: ${CVSROOT_DIRNAME}/first-dir/file2,v
+done
+Checking in file2;
+${CVSROOT_DIRNAME}/first-dir/file2,v <-- file2
+initial revision: 1.1
+done
+HERE
+ dotest branches3-3b "${testcvs} update -d " \
+"${PROG} update: Updating \."
+
+ # First branch gets a commit, then the branch gets deleted.
+ # The purpose of this test is to make sure that even if the
+ # magic branch tag is missing, we see the commits and don't
+ # consider the missing branch tag as eligible for reuse.
+ dotest branches3-4a "${testcvs} -q tag -b br3br1" 'T file1
+T file2'
+ dotest branches3-4b "${testcvs} -q update -r br3br1" ''
+ echo 1:br3br1 >file1
+ dotest branches3-4c "${testcvs} -q ci -m modify" \
+"Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+new revision: 1\.1\.2\.1; previous revision: 1\.1
+done"
+ dotest branches3-4d "${testcvs} tag -d -B br3br1 file1 file2" 'D file1
+D file2'
+
+
+ # Second branch gets no commits.
+ # The purpose of this test is to make sure we respect branches
+ # that don't (yet) have any changes on them.
+ dotest branches3-5a "${testcvs} -q update -r HEAD" 'U file1'
+ echo "about to issue:${testcvs} -q tag -b br3br2" >>$LOGFILE
+ dotest branches3-5b "${testcvs} -q tag -b br3br2" 'T file1
+T file2'
+
+ # Third branch gets several commits so that the 4th
+ # digit of it's revision number is 10 to test that
+ # we don't just consider the highest number in the 4th position.
+ dotest branches3-6a "${testcvs} -q update -r HEAD" ''
+ dotest branches3-6b "${testcvs} -q tag -b br3br3" 'T file1
+T file2'
+
+ dotest branches3-6c "${testcvs} -q update -r br3br3" ''
+ echo 1:br3br3.1 >file1
+ dotest branches3-6d "${testcvs} -q ci -m modify" \
+"Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+new revision: 1\.1\.6\.1; previous revision: 1\.1
+done"
+ echo 1:br3br3.2 >file1
+ dotest branches3-6e "${testcvs} -q ci -m modify" \
+"Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+new revision: 1\.1\.6\.2; previous revision: 1\.1\.6\.1
+done"
+ echo 1:br3br3.3 >file1
+ dotest branches3-6f "${testcvs} -q ci -m modify" \
+"Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+new revision: 1\.1\.6\.3; previous revision: 1\.1\.6\.2
+done"
+ echo 1:br3br3.4 >file1
+ dotest branches3-6g "${testcvs} -q ci -m modify" \
+"Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+new revision: 1\.1\.6\.4; previous revision: 1\.1\.6\.3
+done"
+ echo 1:br3br3.5 >file1
+ dotest branches3-6h "${testcvs} -q ci -m modify" \
+"Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+new revision: 1\.1\.6\.5; previous revision: 1\.1\.6\.4
+done"
+ echo 1:br3br3.6 >file1
+ dotest branches3-6i "${testcvs} -q ci -m modify" \
+"Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+new revision: 1\.1\.6\.6; previous revision: 1\.1\.6\.5
+done"
+ echo 1:br3br3.7 >file1
+ dotest branches3-6i "${testcvs} -q ci -m modify" \
+"Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+new revision: 1\.1\.6\.7; previous revision: 1\.1\.6\.6
+done"
+ echo 1:br3br3.8 >file1
+ dotest branches3-6k "${testcvs} -q ci -m modify" \
+"Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+new revision: 1\.1\.6\.8; previous revision: 1\.1\.6\.7
+done"
+ echo 1:br3br3.9 >file1
+ dotest branches3-6l "${testcvs} -q ci -m modify" \
+"Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+new revision: 1\.1\.6\.9; previous revision: 1\.1\.6\.8
+done"
+ echo 1:br3br3.10 >file1
+ dotest branches3-6m "${testcvs} -q ci -m modify" \
+"Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+new revision: 1\.1\.6\.10; previous revision: 1\.1\.6\.9
+done"
+ # Create a tag on revision 1.1.6.10 so it shows up in RCS symbols
+ dotest branches3-6n "${testcvs} -q tag br3br3_rev_10" 'T file1
+T file2'
+
+ # Create the fourth branch, ensure that it gets the correct magic
+ # branch tag.
+ dotest branches3-7a "${testcvs} -q update -r HEAD" 'U file1'
+ dotest branches3-7b "${testcvs} -q tag -b br3br4" 'T file1
+T file2'
+ dotest branches3-7c "${testcvs} -q update -r br3br4" ''
+ echo 1:br3br4 >file1
+ dotest branches3-7d "${testcvs} -q ci -m modify" \
+"Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+new revision: 1\.1\.8\.1; previous revision: 1\.1
+done"
+
+ # Make a commit on the trunk, then create the fifth branch, make sure we
+ # get the correct revision.
+ # The purpose of this test is to make sure that we're only looking
+ # at the revisions we should be. If we didn't, this branch would end up
+ # being 1.2.0.10 instead of 1.2.0.2
+ cd ..
+ rm -rf first-dir
+ dotest branches3-8a "${testcvs} -q co first-dir" 'U first-dir/file1
+U first-dir/file2'
+ cd first-dir
+ echo 1:HEAD.2 >file1
+ dotest branches3-8b "${testcvs} -q ci -m modify" \
+"Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+new revision: 1\.2; previous revision: 1\.1
+done"
+ dotest branches3-8c "${testcvs} -q tag -b br3br5" 'T file1
+T file2'
+ dotest branches3-8d "${testcvs} -q update -r br3br5" ''
+ echo 1:br3br5 >file1
+ dotest branches3-8e "${testcvs} -q ci -m modify" \
+"Checking in file1;
+${CVSROOT_DIRNAME}/first-dir/file1,v <-- file1
+new revision: 1\.2\.2\.1; previous revision: 1\.2
+done"
+
+
+
+ # clean up after this test
+ cd ..
+
+ if $keep; then
+ echo Keeping ${TESTDIR} and exiting due to --keep
+ exit 0
+ fi
+
+ rm -rf ${CVSROOT_DIRNAME}/first-dir
+ rm -r first-dir
+ ;;
+
+
tagc)
# Test the tag -c option.
mkdir 1; cd 1
_______________________________________________
Bug-cvs mailing list
[email protected]
http://lists.nongnu.org/mailman/listinfo/bug-cvs
