You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ko...@apache.org on 2016/09/06 09:58:00 UTC
svn commit: r1759399 - in /subversion/trunk/subversion: libsvn_subr/stream.c
tests/libsvn_subr/stream-test.c
Author: kotkov
Date: Tue Sep 6 09:58:00 2016
New Revision: 1759399
URL: http://svn.apache.org/viewvc?rev=1759399&view=rev
Log:
Fix the excessive amount of read and seek syscalls in various fsfs operations
that read from disk.
The idea behind the fix is that we provide a special svn_stream_readline()
implementation for APR file-based streams that handles a common case of
LF ("\n") end-of-line sequence with apr_file_gets().
For example, this patch turns the following sequence ...
open("/repo/db/revs/22.pack/manifest", O_RDONLY|O_CLOEXEC) = 4
read(4, "0\n3304\n7139\n12288\n13981\n37151\n59"..., 4096) = 4096
read(4, "5140\n20998974\n21002732\n21004670\n"..., 4096) = 4096
lseek(4, 4020, SEEK_SET) = 4020
read(4, "18102627\n18139220\n18141921\n18147"..., 4096) = 4096
read(4, "2535219\n52537641\n52550290\n525536"..., 4096) = 584
lseek(4, 8043, SEEK_SET) = 8043
read(4, "51452518\n51454318\n51456275\n51457"..., 4096) = 657
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
read(4, "", 4096) = 0
close(4) = 0
...into this:
open("/repo/db/revs/22.pack/manifest", O_RDONLY|O_CLOEXEC) = 4
read(4, "0\n3304\n7139\n12288\n13981\n37151\n59"..., 4096) = 4096
read(4, "5140\n20998974\n21002732\n21004670\n"..., 4096) = 4096
read(4, "131\n52730772\n52744939\n52748273\n5"..., 4096) = 508
read(4, "", 4096) = 0
close(4)
* subversion/libsvn_subr/stream.c
(readline_apr_lf): New.
(readline_apr_generic): New, factored out from ...
(readline_apr_handler): ...here. Call the apr_file_gets()-based routine
when we're looking for an LF end-of-line sequence to avoid unnecessary
syscalls.
* subversion/tests/libsvn_subr/stream-test.c
(test_stream_readline_file): New helper for ...
(test_stream_readline_file_lf, test_stream_readline_file_crlf): ...these
new tests that cover svn_stream_readline() behavior for LF and CRLF
terminated lines.
(test_funcs): Add new tests.
Modified:
subversion/trunk/subversion/libsvn_subr/stream.c
subversion/trunk/subversion/tests/libsvn_subr/stream-test.c
Modified: subversion/trunk/subversion/libsvn_subr/stream.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/stream.c?rev=1759399&r1=1759398&r2=1759399&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/stream.c (original)
+++ subversion/trunk/subversion/libsvn_subr/stream.c Tue Sep 6 09:58:00 2016
@@ -854,18 +854,76 @@ data_available_handler_apr(void *baton,
}
static svn_error_t *
-readline_handler_apr(void *baton,
+readline_apr_lf(apr_file_t *file,
+ svn_stringbuf_t **stringbuf,
+ svn_boolean_t *eof,
+ apr_pool_t *pool)
+{
+ svn_stringbuf_t *buf;
+
+ buf = svn_stringbuf_create_ensure(SVN__LINE_CHUNK_SIZE, pool);
+ while (1)
+ {
+ apr_status_t status;
+
+ status = apr_file_gets(buf->data + buf->len,
+ (int) (buf->blocksize - buf->len),
+ file);
+ buf->len += strlen(buf->data + buf->len);
+
+ if (APR_STATUS_IS_EOF(status))
+ {
+ /* apr_file_gets() keeps the newline; strip it if necessary. */
+ if (buf->len > 0 && buf->data[buf->len - 1] == '\n')
+ svn_stringbuf_chop(buf, 1);
+
+ *eof = TRUE;
+ *stringbuf = buf;
+ return SVN_NO_ERROR;
+ }
+ else if (status != APR_SUCCESS)
+ {
+ const char *fname;
+ svn_error_t *err = svn_io_file_name_get(&fname, file, pool);
+ if (err)
+ fname = NULL;
+ svn_error_clear(err);
+
+ if (fname)
+ return svn_error_wrap_apr(status,
+ _("Can't read a line from file '%s'"),
+ svn_dirent_local_style(fname, pool));
+ else
+ return svn_error_wrap_apr(status,
+ _("Can't read a line from stream"));
+ }
+
+ /* Do we have the EOL? If yes, strip it and return. */
+ if (buf->len > 0 && buf->data[buf->len - 1] == '\n')
+ {
+ svn_stringbuf_chop(buf, 1);
+ *eof = FALSE;
+ *stringbuf = buf;
+ return SVN_NO_ERROR;
+ }
+
+ /* Otherwise, prepare to read the next chunk. */
+ svn_stringbuf_ensure(buf, buf->blocksize + SVN__LINE_CHUNK_SIZE);
+ }
+}
+
+static svn_error_t *
+readline_apr_generic(apr_file_t *file,
svn_stringbuf_t **stringbuf,
const char *eol,
svn_boolean_t *eof,
apr_pool_t *pool)
{
- struct baton_apr *btn = baton;
apr_size_t eol_len = strlen(eol);
apr_off_t offset;
svn_stringbuf_t *buf;
- SVN_ERR(svn_io_file_get_offset(&offset, btn->file, pool));
+ SVN_ERR(svn_io_file_get_offset(&offset, file, pool));
buf = svn_stringbuf_create_ensure(SVN__LINE_CHUNK_SIZE, pool);
while (1)
@@ -883,7 +941,7 @@ readline_handler_apr(void *baton,
else
search_start = buf->data + buf->len - eol_len;
- SVN_ERR(svn_io_file_read_full2(btn->file, buf->data + buf->len,
+ SVN_ERR(svn_io_file_read_full2(file, buf->data + buf->len,
buf->blocksize - buf->len - 1,
&bytes_read, &hit_eof, pool));
buf->len += bytes_read;
@@ -896,7 +954,7 @@ readline_handler_apr(void *baton,
svn_stringbuf_chop(buf, buf->data + buf->len - eol_pos);
/* Seek to the first position behind the EOL. */
offset += (buf->len + eol_len);
- SVN_ERR(svn_io_file_seek(btn->file, APR_SET, &offset, pool));
+ SVN_ERR(svn_io_file_seek(file, APR_SET, &offset, pool));
*eof = FALSE;
*stringbuf = buf;
@@ -914,6 +972,29 @@ readline_handler_apr(void *baton,
}
}
+static svn_error_t *
+readline_handler_apr(void *baton,
+ svn_stringbuf_t **stringbuf,
+ const char *eol,
+ svn_boolean_t *eof,
+ apr_pool_t *pool)
+{
+ struct baton_apr *btn = baton;
+
+ if (eol[0] == '\n' && eol[1] == '\0')
+ {
+ /* Optimize the common case when we're looking for an LF ("\n")
+ end-of-line sequence by using apr_file_gets(). */
+ return svn_error_trace(readline_apr_lf(btn->file, stringbuf,
+ eof, pool));
+ }
+ else
+ {
+ return svn_error_trace(readline_apr_generic(btn->file, stringbuf,
+ eol, eof, pool));
+ }
+}
+
svn_error_t *
svn_stream_open_readonly(svn_stream_t **stream,
const char *path,
Modified: subversion/trunk/subversion/tests/libsvn_subr/stream-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/stream-test.c?rev=1759399&r1=1759398&r2=1759399&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/stream-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/stream-test.c Tue Sep 6 09:58:00 2016
@@ -870,6 +870,136 @@ test_stream_checksum(apr_pool_t *pool)
return SVN_NO_ERROR;
}
+static svn_error_t *
+test_stream_readline_file(const char *testname,
+ const char *eol,
+ apr_pool_t *pool)
+{
+ const char *tmp_dir;
+ const char *tmp_file;
+ svn_stream_t *stream;
+ svn_stringbuf_t *line;
+ svn_boolean_t eof;
+ static const char long_line[] =
+ "The quick brown fox jumps over the lazy dog, and "
+ "jackdaws love my big sphinx of quartz, and "
+ "pack my box with five dozen liquor jugs.";
+
+ SVN_ERR(svn_dirent_get_absolute(&tmp_dir, testname, pool));
+ SVN_ERR(svn_io_remove_dir2(tmp_dir, TRUE, NULL, NULL, pool));
+ SVN_ERR(svn_io_make_dir_recursively(tmp_dir, pool));
+ svn_test_add_dir_cleanup(tmp_dir);
+
+ /* Test 1: Read empty file. */
+ tmp_file = svn_dirent_join(tmp_dir, "empty", pool);
+ SVN_ERR(svn_io_file_create(tmp_file, "", pool));
+ SVN_ERR(svn_stream_open_readonly(&stream, tmp_file, pool, pool));
+
+ SVN_ERR(svn_stream_readline(stream, &line, eol, &eof, pool));
+ SVN_TEST_ASSERT(line->len == 0);
+ SVN_TEST_STRING_ASSERT(line->data, "");
+ SVN_TEST_ASSERT(eof);
+
+ SVN_ERR(svn_stream_readline(stream, &line, eol, &eof, pool));
+ SVN_TEST_ASSERT(line->len == 0);
+ SVN_TEST_STRING_ASSERT(line->data, "");
+ SVN_TEST_ASSERT(eof);
+
+ SVN_ERR(svn_stream_close(stream));
+
+ /* Test 2: Read empty line. */
+ tmp_file = svn_dirent_join(tmp_dir, "empty-line", pool);
+ SVN_ERR(svn_io_file_create(tmp_file, eol, pool));
+ SVN_ERR(svn_stream_open_readonly(&stream, tmp_file, pool, pool));
+
+ SVN_ERR(svn_stream_readline(stream, &line, eol, &eof, pool));
+ SVN_TEST_ASSERT(line->len == 0);
+ SVN_TEST_STRING_ASSERT(line->data, "");
+ SVN_TEST_ASSERT(!eof);
+
+ SVN_ERR(svn_stream_readline(stream, &line, eol, &eof, pool));
+ SVN_TEST_ASSERT(line->len == 0);
+ SVN_TEST_STRING_ASSERT(line->data, "");
+ SVN_TEST_ASSERT(eof);
+
+ SVN_ERR(svn_stream_close(stream));
+
+ /* Test 3: Read two lines. */
+ tmp_file = svn_dirent_join(tmp_dir, "lines", pool);
+ SVN_ERR(svn_io_file_create(tmp_file,
+ apr_pstrcat(pool,
+ "first", eol, "second", eol,
+ SVN_VA_NULL),
+ pool));
+ SVN_ERR(svn_stream_open_readonly(&stream, tmp_file, pool, pool));
+
+ SVN_ERR(svn_stream_readline(stream, &line, eol, &eof, pool));
+ SVN_TEST_ASSERT(line->len == 5);
+ SVN_TEST_STRING_ASSERT(line->data, "first");
+ SVN_TEST_ASSERT(!eof);
+
+ SVN_ERR(svn_stream_readline(stream, &line, eol, &eof, pool));
+ SVN_TEST_ASSERT(line->len == 6);
+ SVN_TEST_STRING_ASSERT(line->data, "second");
+ SVN_TEST_ASSERT(!eof);
+
+ SVN_ERR(svn_stream_readline(stream, &line, eol, &eof, pool));
+ SVN_TEST_ASSERT(line->len == 0);
+ SVN_TEST_STRING_ASSERT(line->data, "");
+ SVN_TEST_ASSERT(eof);
+
+ SVN_ERR(svn_stream_close(stream));
+
+ /* Test 4: Content without end-of-line. */
+ tmp_file = svn_dirent_join(tmp_dir, "no-eol", pool);
+ SVN_ERR(svn_io_file_create(tmp_file, "text", pool));
+ SVN_ERR(svn_stream_open_readonly(&stream, tmp_file, pool, pool));
+
+ SVN_ERR(svn_stream_readline(stream, &line, eol, &eof, pool));
+ SVN_TEST_ASSERT(line->len == 4);
+ SVN_TEST_STRING_ASSERT(line->data, "text");
+ SVN_TEST_ASSERT(eof);
+
+ SVN_ERR(svn_stream_close(stream));
+
+ /* Test 5: Read long line. */
+ tmp_file = svn_dirent_join(tmp_dir, "long-line", pool);
+ SVN_ERR(svn_io_file_create(tmp_file,
+ apr_pstrcat(pool, long_line, eol, SVN_VA_NULL),
+ pool));
+ SVN_ERR(svn_stream_open_readonly(&stream, tmp_file, pool, pool));
+
+ SVN_ERR(svn_stream_readline(stream, &line, eol, &eof, pool));
+ SVN_TEST_ASSERT(line->len == strlen(long_line));
+ SVN_TEST_STRING_ASSERT(line->data, long_line);
+ SVN_TEST_ASSERT(!eof);
+
+ SVN_ERR(svn_stream_readline(stream, &line, eol, &eof, pool));
+ SVN_TEST_ASSERT(line->len == 0);
+ SVN_TEST_STRING_ASSERT(line->data, "");
+ SVN_TEST_ASSERT(eof);
+
+ SVN_ERR(svn_stream_close(stream));
+
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+test_stream_readline_file_lf(apr_pool_t *pool)
+{
+ SVN_ERR(test_stream_readline_file("test_stream_readline_file_lf",
+ "\n", pool));
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+test_stream_readline_file_crlf(apr_pool_t *pool)
+{
+ SVN_ERR(test_stream_readline_file("test_stream_readline_file_crlf",
+ "\r\n", pool));
+ return SVN_NO_ERROR;
+}
+
/* The test table. */
static int max_threads = 1;
@@ -903,6 +1033,10 @@ static struct svn_test_descriptor_t test
"test compression for streams without partial read"),
SVN_TEST_PASS2(test_stream_checksum,
"test svn_stream_contents_checksum()"),
+ SVN_TEST_PASS2(test_stream_readline_file_lf,
+ "test reading LF-terminated lines from file"),
+ SVN_TEST_PASS2(test_stream_readline_file_crlf,
+ "test reading CRLF-terminated lines from file"),
SVN_TEST_NULL
};