All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: git@vger.kernel.org
Cc: David Barr <david.barr@cordelta.com>,
	Thomas Rast <trast@student.ethz.ch>,
	Ramkumar Ramachandra <artagnon@gmail.com>
Subject: [PATCH 2/4] vcs-svn: replace buffer_read_string memory pool with a strbuf
Date: Fri, 24 Dec 2010 02:17:00 -0600	[thread overview]
Message-ID: <20101224081700.GC29681@burratino> (raw)
In-Reply-To: <20101224080505.GA29681@burratino>

Date: Sat, 6 Nov 2010 12:01:28 -0500

obj_pool is inherently global and does not use the standard growing
factor alloc_nr, which makes it feel out of place in the git codebase.
Plus it is overkill for this application: all that is needed is a
buffer that can grow between requests to accomodate larger strings.
Use a strbuf instead.

As a side effect, this improves the error handling: allocation
failures will result in a clean exit instead of segfaults.  It would
be nice to add a test case (using ulimit or failmalloc) but that can
wait for another day.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Requires jn/thinner-wrapper (from master) if contrib/svn-fe/svn-fe is
to build without linking to libz et al.

The initial size of the per-line buffer shrinks from 4096 to 0 (well,
maybe 16 or so).  strbuf_fread is not inline.  I haven't looked into
the effect on performance from these changes.

I find obj_pool tricky to use correctly (see 3c93983, vcs-svn: fix
intermittent repo_tree corruption, 2010-12-05 for example) so I look
forward to eliminating obj_pool from the vcs-svn/ dir altogether.
Excitingly enough, David has already done that, it seems[1].

[1] git://github.com/barrbrain/git.git vcs-svn-incremental

 vcs-svn/line_buffer.c |   16 ++++++----------
 1 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/vcs-svn/line_buffer.c b/vcs-svn/line_buffer.c
index f22c94f..6f32f28 100644
--- a/vcs-svn/line_buffer.c
+++ b/vcs-svn/line_buffer.c
@@ -5,15 +5,13 @@
 
 #include "git-compat-util.h"
 #include "line_buffer.h"
-#include "obj_pool.h"
+#include "strbuf.h"
 
 #define LINE_BUFFER_LEN 10000
 #define COPY_BUFFER_LEN 4096
 
-/* Create memory pool for char sequence of known length */
-obj_pool_gen(blob, char, 4096)
-
 static char line_buffer[LINE_BUFFER_LEN];
+static struct strbuf blob_buffer = STRBUF_INIT;
 static FILE *infile;
 
 int buffer_init(const char *filename)
@@ -58,11 +56,9 @@ char *buffer_read_line(void)
 
 char *buffer_read_string(uint32_t len)
 {
-	char *s;
-	blob_free(blob_pool.size);
-	s = blob_pointer(blob_alloc(len + 1));
-	s[fread(s, 1, len, infile)] = '\0';
-	return ferror(infile) ? NULL : s;
+	strbuf_reset(&blob_buffer);
+	strbuf_fread(&blob_buffer, len, infile);
+	return ferror(infile) ? NULL : blob_buffer.buf;
 }
 
 void buffer_copy_bytes(uint32_t len)
@@ -94,5 +90,5 @@ void buffer_skip_bytes(uint32_t len)
 
 void buffer_reset(void)
 {
-	blob_reset();
+	strbuf_release(&blob_buffer);
 }
-- 
1.7.2.3.554.gc9b5c.dirty

  parent reply	other threads:[~2010-12-24  8:17 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-24  8:05 [PATCH 0/4] teach vcs-svn/line_buffer to handle multiple input files Jonathan Nieder
2010-12-24  8:08 ` [PATCH 1/4] vcs-svn: eliminate global byte_buffer Jonathan Nieder
2010-12-24  8:17 ` Jonathan Nieder [this message]
2010-12-24  8:18 ` [PATCH 3/4] vcs-svn: collect line_buffer data in a struct Jonathan Nieder
2010-12-24  8:28 ` [PATCH 4/4] vcs-svn: teach line_buffer to handle multiple input files Jonathan Nieder
2011-01-03  0:49 ` [PATCH 0/4] teach vcs-svn/line_buffer " Jonathan Nieder
2011-01-03  0:50   ` [PATCH 5/8] vcs-svn: make test-line-buffer input format more flexible Jonathan Nieder
2011-01-03  0:51   ` [PATCH 6/8] tests: give vcs-svn/line_buffer its own test script Jonathan Nieder
2011-01-03  0:52   ` [PATCH 7/8] vcs-svn: tweak test-line-buffer to not assume line-oriented input Jonathan Nieder
2011-01-03  1:07   ` [PATCH 8/8] t0081 (line-buffer): add buffering tests Jonathan Nieder
2011-01-03  1:34     ` Jonathan Nieder
2011-01-03  3:03   ` [PATCHES 9-12/12] line_buffer: more wrappers around stdio functions Jonathan Nieder
2011-01-03  3:05     ` [PATCH 09/12] vcs-svn: add binary-safe read function Jonathan Nieder
2011-01-03  3:06     ` [PATCH 10/12] vcs-svn: allow character-oriented input Jonathan Nieder
2011-01-03  3:09     ` [PATCH 11/12] vcs-svn: allow input from file descriptor Jonathan Nieder
2011-01-03  3:10     ` [PATCH 12/12] vcs-svn: teach line_buffer about temporary files Jonathan Nieder
2011-01-22  6:42       ` [FYI/PATCH] vcs-svn: give control over temporary file names Jonathan Nieder
2011-02-26 11:44 ` [PULL svn-fe] fast-import 'ls', line-buffer changes Jonathan Nieder
2011-02-26 12:03   ` David Michael Barr
2011-02-28  6:15   ` Junio C Hamano
2011-02-28 21:32     ` [PATCH svn-fe] fast-import: make code "-Wpointer-arith" clean Jonathan Nieder
2011-02-28 21:36       ` Sverre Rabbelier
2011-02-28 22:05         ` Junio C Hamano
2011-02-28 23:15       ` Jonathan Nieder
2011-03-01  0:41         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101224081700.GC29681@burratino \
    --to=jrnieder@gmail.com \
    --cc=artagnon@gmail.com \
    --cc=david.barr@cordelta.com \
    --cc=git@vger.kernel.org \
    --cc=trast@student.ethz.ch \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.