bugGNU RCS - Bugs: bug #40200, performance regression in newer...

 
 

bug #40200: performance regression in newer versions of RCS

Submitted by:  Andrew J. Schorr <ajschorr>
Submitted on:  Mon 07 Oct 2013 09:48:32 PM UTC  
 
Category: NoneSeverity: 2 - Minor
Item Group: NoneStatus: Postponed
Privacy: PublicAssigned to: Thien-Thi Nguyen <ttn>
Open/Closed: Open

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Tue 22 Oct 2013 09:04:30 AM UTC, comment #11:

Thanks for testing. I will change the status now to Postponed, as the rest of the performance difference between 5.7 and current code is mostly due to architectural changes (i believe), and the search for any implmentation-specific performance problems will not be worth the trouble.

That said, if anyone (reading this) wants to performance profile 5.7 vs current code, then this is the right thread to post findings. Maybe i'm wrong (happens often enough) and we can indeed fix some things before the checksums are in place.

Thien-Thi Nguyen <ttn>
Project AdministratorIn charge of this item.
Mon 21 Oct 2013 06:35:28 PM UTC, comment #10:

I applied that patch to 5.9.1. With the patch, it takes
the same amount of time as in my previous test with a large value for RCS_MEM_LIMIT.

Andrew J. Schorr <ajschorr>
Sun 20 Oct 2013 09:39:28 AM UTC, comment #9:

I understand. Please find attached a patch, a subset of the one i just pushed:

(The full change also updates the documentation.) Could you please try it? I'm changing the status to Ready for Test, as well.

(file #29423)

Thien-Thi Nguyen <ttn>
Project AdministratorIn charge of this item.
Fri 18 Oct 2013 04:46:28 PM UTC, comment #8:

I can give it a try. My actual usage case is a script
that loops over each saved version of the file. So if
5.9.1 is 2.5 times as slow as 5.7 (as with the
unlimited RCS_MEM_LIMIT), I still may not want to use it.

Andrew J. Schorr <ajschorr>
Fri 18 Oct 2013 04:34:43 PM UTC, comment #7:

Yes, 5.7 behavior is/was consistent w/ "unlimited" RCS_MEM_LIMIT. Thanks for the feedback. I will install that change shortly. Would you be willing to try a patch against 5.9.1?

Thien-Thi Nguyen <ttn>
Project AdministratorIn charge of this item.
Fri 18 Oct 2013 03:58:28 PM UTC, comment #6:

Was the behavior of version 5.7 consistent with setting RCS_MEM_LIMIT to unlimited? If yes, then that sounds good.
If you change the arbitrary cutoff from 256K to 100M, it still
runs the risk of a severe performance regression when people
go over the limit. It never even occurred to me to check
the man page. I have been using RCS for many years (decades?),
so why would I suddenly wonder if I were using it incorrectly?

IMHO, these are supposed to be simple, fast, basic, and reliable
tools. The user shouldn't have to read the man page to
figure out how to configure it to work with acceptable performance.
If I want complexity, I'll use git!

Andrew J. Schorr <ajschorr>
Fri 18 Oct 2013 03:54:45 PM UTC, comment #5:

Good. Given this (expected) result, i think the nature of the bug starts to appear as "insufficient documentation" and/or "bad (out of touch w/ modern systems) defaults", both of which strike me as less than grave in severity. So that explains the drop in severity to Minor.

Still, aside from making ‘RCS_MEM_LIMIT’ more prominant in the docs (which i suppose few read, anyway), there is something we can do in the bug/bugfix frame of mind: find a better default value than the archaic 256. How does 100 megabytes sound? Another idea is to change the interpretation of unspecified ‘RCS_MEM_LIMIT’ from a default value to "unlimited". Maybe that would be better?

Thien-Thi Nguyen <ttn>
Project AdministratorIn charge of this item.
Fri 18 Oct 2013 02:01:43 PM UTC, comment #4:

Yes, that helps a lot. Here is the performance of 5.7:

bash-4.2$ co -V
RCS version 5.7
bash-4.2$ time co -p1.1 Index | wc
RCS/Index,v --> standard output
revision 1.1
13212 183100 971257

real 0m0.299s
user 0m0.285s
sys 0m0.026s

Here is 5.9.1 without setting RCS_MEM_LIMIT:

bash-4.2$ co --version
co (GNU RCS) 5.9.1
Copyright (C) 2010-2013 Thien-Thi Nguyen
Copyright (C) 1990-1995 Paul Eggert
Copyright (C) 1982,1988,1989 Walter F. Tichy, Purdue CS
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
bash-4.2$ time co -p1.1 Index | wc
RCS/Index,v --> standard output
revision 1.1
13212 183100 971257

real 1m42.994s
user 1m37.399s
sys 0m4.564s

And with RCS_MEM_LIMIT:

bash-4.2$ ls -l RCS/Index,v
-r--r--r-- 1 usdb ead 54975095 Oct 17 18:31 RCS/Index,v
bash-4.2$ export RCS_MEM_LIMIT=54272
bash-4.2$ time co -p1.1 Index | wc
RCS/Index,v --> standard output
revision 1.1
13212 183100 971257

real 0m0.773s
user 0m0.759s
sys 0m0.027s

It is still slower than 5.7, but it's not horrendous.

Andrew J. Schorr <ajschorr>
Fri 18 Oct 2013 10:46:31 AM UTC, comment #3:

BTW, RCS consults env var ‘RCS_MEM_LIMIT’ to determine whether or not to fall back to (slow) stdio mode (which uses temporary files) from the default in-memory (mmap et al) mode. The default value is "256" (kilobytes), which is probably too low these days on most workstations.

Please try that command with ‘RCS_MEM_LIMIT=N’ in the environment, where N is the size of the comma-v file, rounded up to the next megabyte, but expressed in kilobytes. For example, if the comma-v file is 1010456 bytes, then the rounded-up value is 1048576 (1 megabyte) and N will be 1024.

Does that help?

Thien-Thi Nguyen <ttn>
Project AdministratorIn charge of this item.
Fri 18 Oct 2013 09:22:20 AM UTC, comment #2:

Another factor, probably more important in this (-p) case, is that 5.7 co managed do its work w/o temporary files, whereas current co creates a temporary file for each revision. This can be seen w/ strace(8), for example. Well, that's clearly bogus and should be fixed!

So, i'm changing the status to In Progress and will followup later w/ more analysis and a fix.

Thien-Thi Nguyen <ttn>
Project AdministratorIn charge of this item.
Tue 15 Oct 2013 07:44:37 AM UTC, comment #1:

Prior to 5.8, RCS was fast and loose with the trailing part of the comma-v file, leading in some cases to long-standing undetected corruption. As of 5.8, each access involves an integrity check, which involves scanning the entire file and results in the slowdown you see. So yes, i can confirm that the performance regression is known.

The way to achieve both integrity and performance is to use some kind of checksum so that the scan need not be as detailed. That feature is still in the design stage, however.

Thien-Thi Nguyen <ttn>
Project AdministratorIn charge of this item.
Mon 07 Oct 2013 09:48:32 PM UTC, original submission:

Hi,

On a newly installed Fedora 19 system running version 5.9 of RCS, I noticed that it takes an incredibly long time to run "co -p1.1 <filename>" on a file that has 3585 revisions in its history. It takes over 1 minute. By comparison, on Fedora 16 with version 5.7 of RCS, it takes less than 1 second. I built 5.7 on the new O/S to confirm that it's not an O/S problem, and there are no performance problems with version 5.7 running on Fedora 19. I started to run "git bisect" to find the problem, but I ran into trouble, since intermediate versions would not build. So I gave up and installed 5.7 on my Fedora 19 systems.

N.B. I found the same performance problems in versions 5.8.1 and 5.8.2.

I opened this Fedora bug as well.
https://bugzilla.redhat.com/show_bug.cgi?id=1016308

Regards,
Andy

Andrew J. Schorr <ajschorr>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #29423:  unlim.diff added by ttn (3kB - text/x-diff)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by ttn (Posted a comment)
  • -unavailable- added by ajschorr (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 7 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 22 Oct 2013 09:04:30 AM UTCttnStatusReady For Test=>Postponed
    Sun 20 Oct 2013 10:17:39 AM UTCttnStatusIn Progress=>Ready For Test
    Sun 20 Oct 2013 09:39:28 AM UTCttnAttached File-=>Added unlim.diff, #29423
    Fri 18 Oct 2013 03:55:05 PM UTCttnSeverity3 - Normal=>2 - Minor
    Fri 18 Oct 2013 09:22:20 AM UTCttnStatusConfirmed=>In Progress
    Tue 15 Oct 2013 07:44:37 AM UTCttnStatusNone=>Confirmed
      Assigned toNone=>ttn

    Back to the top


    Powered by Savane 3.1-cleanup