@@ -99,6 +99,79 @@ NFS, Amazon S3 ...) and chunking for resumable downloads.
9999
100100### Reviews
101101
102+ * [ Speeding up strbuf_getwholeline()] ( http://thread.gmane.org/gmane.comp.version-control.git/266789/focus=266796 )
103+
104+ Jeff King, alias Peff, posted a patch series to address speed
105+ regressions when accessing the packed-refs file. This lead to
106+ discussions about ways to speed up reading lines in a file.
107+
108+ The packed-ref file has been created a long time ago to speed up
109+ dealing with refs. The original way to store refs, which is called the
110+ "loosed ref" format uses one file per ref to store its content and is
111+ still often used by git. But when the number of refs increases, it
112+ becomes much faster to have as much information as possible in a
113+ single file. That's the purpose of the packed-ref file.
114+
115+ Peff discovered that one of his own commit that switched from fgets()
116+ to strbuf_getwholeline() to read the packed-ref file was in part
117+ responsible for a big slowdown.
118+
119+ strbuf_getwholeline() is part of the Git strbuf API that is used for a
120+ lot of string related functions. And strbuf_getwholeline() used the
121+ getc() function to get each character one by one until the end of each
122+ line, like this:
123+
124+ ```
125+ while ((ch = getc(fp)) != EOF) {
126+ ...
127+ if (ch == '\n')
128+ break;
129+ }
130+ ```
131+
132+ But it appears that it isn't very efficient. It is also problematic to
133+ use fgets() inside strbuf_getwholeline() as strbuf_getwholeline() is
134+ used in some parts of the Git codebase to read lines that can contain
135+ the NUL character and fgets() would not read after the NUL. (And yeah
136+ working around that is not easy either.)
137+
138+ So Peff came up with the following explanation and solution to the
139+ problem:
140+
141+ > strbuf_getwholeline calls getc in a tight loop. On modern
142+ > libc implementations, the stdio code locks the handle for
143+ > every operation, which means we are paying a significant
144+ > overhead. We can get around this by locking the handle for
145+ > the whole loop and using the unlocked variant.
146+
147+ His patch does basically:
148+
149+ ```
150+ + flockfile(fp);
151+ + while ((ch = getc_unlocked(fp)) != EOF) {
152+ ...
153+ if (ch == '\n')
154+ break;
155+ }
156+ + funlockfile(fp);
157+ ```
158+
159+ Duy Nguyen suggested instead to avoid any FILE* interface and either
160+ mmap the entire file, or read (with bufferring) from a file
161+ descriptor, as Git already does to read the index-pack file. But Peff
162+ said that it would be very inefficient too, and that there are no good
163+ NUL safe function to read from a file descriptor.
164+
165+ Junio wondered if it would be worth it to have callers that need to
166+ handle NUL characters to pass a flag, so that the default
167+ implementation would still be fast.
168+
169+ Eventually Rasmus Villemoes suggested using
170+ [ getdelim()] ( http://pubs.opengroup.org/stage7tc1/functions/getdelim.html )
171+ when POSIX 2008 is supported and so far this looks like a workable
172+ solution.
173+
174+
102175### Support
103176
104177## Releases
0 commit comments