You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2012/09/07 01:32:12 UTC
svn commit: r1381800 - /subversion/trunk/subversion/libsvn_subr/io.c
Author: stefan2
Date: Thu Sep 6 23:32:11 2012
New Revision: 1381800
URL: http://svn.apache.org/viewvc?rev=1381800&view=rev
Log:
Re-implement svn_io_read_length_line as this is one of the
most-called functions in SVN. Instead of reading data a byte
at a time, read 128 byte chunks and scan those.
* subversion/libsvn_subr/io.c
(svn_io_read_length_line): reimplement
Modified:
subversion/trunk/subversion/libsvn_subr/io.c
Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1381800&r1=1381799&r2=1381800&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Thu Sep 6 23:32:11 2012
@@ -3427,30 +3427,60 @@ svn_error_t *
svn_io_read_length_line(apr_file_t *file, char *buf, apr_size_t *limit,
apr_pool_t *pool)
{
+ /* variables */
+ apr_size_t total_read = 0;
+ svn_boolean_t eof = FALSE;
const char *name;
svn_error_t *err;
- apr_size_t i;
- char c;
+ apr_size_t buf_size = *limit;
- for (i = 0; i < *limit; i++)
+ while (buf_size > 0)
{
- SVN_ERR(svn_io_file_getc(&c, file, pool));
- /* Note: this error could be APR_EOF, which
- is totally fine. The caller should be aware of
- this. */
-
- if (c == '\n')
+ /* read a fair chunk of data at once. But don't get too ambitious
+ * as that would result in too much waste. Also make sure we can
+ * put a NUL after the last byte read.
+ */
+ apr_size_t to_read = buf_size < 129 ? buf_size - 1 : 128;
+ apr_size_t bytes_read = 0;
+ char *eol;
+
+ /* read data block (or just a part of it) */
+ SVN_ERR(svn_io_file_read_full2(file, buf, to_read,
+ &bytes_read, &eof, pool));
+
+ /* look or a newline char */
+ buf[bytes_read] = 0;
+ eol = strchr(buf, '\n');
+ if (eol)
{
- buf[i] = '\0';
- *limit = i;
+ apr_off_t offset = (eol + 1 - buf) - bytes_read;
+
+ *eol = 0;
+ *limit = total_read + (eol - buf);
+
+ /* correct the file pointer:
+ * appear as though we just had read the newline char
+ */
+ SVN_ERR(svn_io_file_seek(file, APR_CUR, &offset, pool));
+
return SVN_NO_ERROR;
}
- else
+ else if (eof)
{
- buf[i] = c;
+ /* no EOL found but we hit the end of the file.
+ * Generate a nice EOF error object and return it.
+ */
+ char dummy;
+ SVN_ERR(svn_io_file_getc(&dummy, file, pool));
}
+
+ /* next data chunk */
+ buf_size -= bytes_read;
+ buf += bytes_read;
+ total_read += bytes_read;
}
+ /* buffer limit has been exceeded without finding the EOL */
err = svn_io_file_name_get(&name, file, pool);
if (err)
name = NULL;
Re: svn commit: r1381800 - /subversion/trunk/subversion/libsvn_subr/io.c
Posted by Philip Martin <ph...@wandisco.com>.
Paul Burba <pt...@gmail.com> writes:
> Anybody else seeing this?
>
> I haven't figured out why yet, but r1381800 is causing failures on my
> Windows box, all similar to this:
>
> C:\SVN\src-trunk>win-tests.py -d -c --test=basic#1 --log-to-stdout
> --log-level=DEBUG
> Testing Debug configuration on local repository.
> START: basic_tests.py
> I: CMD: svnadmin.exe create svn-test-work\local_tmp\repos
> --bdb-txn-nosync --fs-type=fsfs
> I: <TIME = 1.990000>
> I: CMD: svn.exe import -m "Log message for revision 1."
> svn-test-work\local_tmp\greekfiles
> file:///C:/SVN/src-trunk/Debug/subversion/tests/cmdline/svn-test-work
> /local_tmp/repos --config-dir
> C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
> --password rayjandom --no-auth-cache --username jra
> ndom
> I: CMD: C:\SVN\src-trunk\Debug\subversion\svn\svn.exe import -m "Log
> message for revision 1." svn-test-work\local_tmp\greekfiles
> file:///C:/SVN/src-trunk/Debug/
> subversion/tests/cmdline/svn-test-work/local_tmp/repos --config-dir
> C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
> --password ra
> yjandom --no-auth-cache --username jrandom exited with 1
> I: <TIME = 4.514000>
> I: ..\..\..\subversion\svn\import-cmd.c:128: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_client\commit.c:976: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_client\commit.c:712: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_client\commit.c:442: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_client\commit.c:535: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_repos\commit.c:348: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_fs\fs-loader.c:1123: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_fs_fs\tree.c:1827: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_fs_fs\tree.c:675: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_fs_fs\dag.c:1147: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_fs_fs\dag.c:315: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5340: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5291: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5136: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_subr\stream.c:143: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:4905: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:4839: (apr_err=70014)
> I: ..\..\..\subversion\libsvn_subr\io.c:3215: (apr_err=70014)
> I: svn: E070014: Can't read file
> 'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\repos\db\revs\0\0':
> End of file found
> W: ..\..\..\subversion\svn\import-cmd.c:128: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_client\commit.c:976: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_client\commit.c:712: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_client\commit.c:442: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_client\commit.c:535: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_repos\commit.c:348: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_fs\fs-loader.c:1123: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_fs_fs\tree.c:1827: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_fs_fs\tree.c:675: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_fs_fs\dag.c:1147: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_fs_fs\dag.c:315: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5340: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5291: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5136: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_subr\stream.c:143: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:4905: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:4839: (apr_err=70014)
> W: ..\..\..\subversion\libsvn_subr\io.c:3215: (apr_err=70014)
> W: svn: E070014: Can't read file
> 'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\repos\db\revs\0\0':
> End of file found
> END: basic_tests.py
> ELAPSED: basic_tests.py 0:00:06.609000
valgrind on my Linux box reports:
W: ==23808== Conditional jump or move depends on uninitialised value(s)
W: ==23808== at 0x5A21A6C: first_non_ascii_char_cstring (utf_validate.c:312)
W: ==23808== by 0x5A21BB1: svn_utf__cstring_is_valid (utf_validate.c:352)
W: ==23808== by 0x5A20D3F: check_cstring_utf8 (utf.c:720)
W: ==23808== by 0x5A211BD: svn_utf_cstring_to_utf8 (utf.c:833)
W: ==23808== by 0x417C5E: sub_main (main.c:1994)
W: ==23808== by 0x419A68: main (main.c:2735)
W: ==23808== Conditional jump or move depends on uninitialised value(s)
W: ==23808== at 0x5A21A6C: first_non_ascii_char_cstring (utf_validate.c:312)
W: ==23808== by 0x5A21BB1: svn_utf__cstring_is_valid (utf_validate.c:352)
W: ==23808== by 0x5A20D3F: check_cstring_utf8 (utf.c:720)
W: ==23808== by 0x5A21591: svn_utf_cstring_from_utf8 (utf.c:942)
W: ==23808== by 0x5A08BA0: svn_path_cstring_from_utf8 (path.c:1140)
W: ==23808== by 0x59F350F: cstring_from_utf8 (io.c:167)
W: ==23808== by 0x59F3718: io_check_path (io.c:290)
W: ==23808== by 0x59F38CD: svn_io_check_resolved_path (io.c:336)
W: ==23808== by 0x59E7A5E: svn_config_ensure (config_file.c:575)
W: ==23808== by 0x418312: sub_main (main.c:2159)
W: ==23808== by 0x419A68: main (main.c:2735)
W: ==23808== Conditional jump or move depends on uninitialised value(s)
W: ==23808== at 0x5A21A6C: first_non_ascii_char_cstring (utf_validate.c:312)
W: ==23808== by 0x5A21BB1: svn_utf__cstring_is_valid (utf_validate.c:352)
W: ==23808== by 0x6A7368A: svn_fs__path_valid (fs-loader.c:384)
W: ==23808== by 0x6A758D6: svn_fs_make_dir (fs-loader.c:1122)
W: ==23808== by 0x6836867: add_file_or_directory (commit.c:348)
W: ==23808== by 0x6836E50: add_directory (commit.c:482)
W: ==23808== by 0x4E424D2: import_dir (commit.c:534)
W: ==23808== by 0x4E421A0: import_children (commit.c:437)
W: ==23808== by 0x4E42B8A: import (commit.c:708)
W: ==23808== by 0x4E43752: svn_client_import5 (commit.c:969)
W: ==23808== by 0x411312: svn_cl__import (import-cmd.c:115)
W: ==23808== by 0x41985C: sub_main (main.c:2680)
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: svn commit: r1381800 - /subversion/trunk/subversion/libsvn_subr/io.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
One minute after my fix ;)
On Sat, Sep 8, 2012 at 12:35 AM, Paul Burba <pt...@gmail.com> wrote:
> On Fri, Sep 7, 2012 at 3:29 PM, Julian Foad <ju...@btopenworld.com>
> wrote:
> > (Just forwarding this thread to Stefan.)
> >
> >
> > - Julian
> >
> >
> > I (Julian Foad) wrote:
> >
> >> Paul Burba wrote:
> >>
> >>> On Thu, Sep 6, 2012 at 7:32 PM, <st...@apache.org> wrote:
> >>>> URL: http://svn.apache.org/viewvc?rev=1381800&view=rev
> >>>> Log:
> >>>> Re-implement svn_io_read_length_line as this is one of the
> >>>> most-called functions in SVN. Instead of reading data a byte
> >>>> at a time, read 128 byte chunks and scan those.
> >> [...]
> >>>>
> ==============================================================================
> >>>> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> >>>> +++ subversion/trunk/subversion/libsvn_subr/io.c Thu Sep 6
> >>>> @@ -3427,30 +3427,60 @@ svn_error_t *
> >>>> svn_io_read_length_line(apr_file_t *file, char *buf, apr_size_t
> *limit,
> >>>> apr_pool_t *pool)
> >>>> {
> >>>> + /* variables */
> >
> > p.s. Redundant comment there.
> >
> >>>> + apr_size_t total_read = 0;
> >>>> + svn_boolean_t eof = FALSE;
> >>>> const char *name;
> >>>> svn_error_t *err;
> >>>> - apr_size_t i;
> >>>> - char c;
> >>>> + apr_size_t buf_size = *limit;
> >>>>
> >>>> - for (i = 0; i < *limit; i++)
> >>>> + while (buf_size > 0)
> >>>> {
> >>>> - SVN_ERR(svn_io_file_getc(&c, file, pool));
> >>>> - /* Note: this error could be APR_EOF, which
> >>>> - is totally fine. The caller should be aware of
> >>>> - this. */
> >>>> -
> >>>> - if (c == '\n')
> >>>> + /* read a fair chunk of data at once. But don't get too
> ambitious
> >>>> + * as that would result in too much waste. Also make sure we
> can
> >>>> + * put a NUL after the last byte read.
> >>>> + */
> >>>> + apr_size_t to_read = buf_size < 129 ? buf_size - 1 : 128;
> >>>> + apr_size_t bytes_read = 0;
> >>>> + char *eol;
> >>>> +
> >>>> + /* read data block (or just a part of it) */
> >>>> + SVN_ERR(svn_io_file_read_full2(file, buf, to_read,
> >>>> + &bytes_read, &eof, pool));
> >>>> +
> >>>> + /* look or a newline char */
> >>>> + buf[bytes_read] = 0;
> >>>> + eol = strchr(buf, '\n');
> >>>> + if (eol)
> >>>> {
> >>>> - buf[i] = '\0';
> >>>> - *limit = i;
> >>>> + apr_off_t offset = (eol + 1 - buf) - bytes_read;
>
> Here is the problem: apr_off_t is unsigned and in some cases we are
> trying to calculate a negative offset to pass to svn_io_file_seek.
> The attached patch fixes this and all tests pass with it, but I'd
> appreciate another set of eyes before committing.
>
> [[[
> Fix file seek breakage introduced in r1381800.
>
> See also discussion at http://svn.haxx.se/dev/archive-2012-09/0114.shtml
>
> * subversion/libsvn_subr/io.c
> (svn_io_read_length_line): Move the read/write file offset to an
> absolute offset rather than trying to applying a negative offset to
> the current offset, thus preventing the mayhem that comes with
> assigning a negative offset value to an unsigned apr_off_t.
> ]]]
>
> --
> Paul T. Burba
> CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
> Skype: ptburba
>
> >>>> + *eol = 0;
> >>>> + *limit = total_read + (eol - buf);
> >>>> +
> >>>> + /* correct the file pointer:
> >>>> + * appear as though we just had read the newline char
> >>>> + */
> >>>> + SVN_ERR(svn_io_file_seek(file, APR_CUR, &offset, pool));
> >>>> +
> >>>> return SVN_NO_ERROR;
> >>>> }
> >>>> - else
> >>>> + else if (eof)
> >>>> {
> >>>> - buf[i] = c;
> >>>> + /* no EOL found but we hit the end of the file.
> >>>> + * Generate a nice EOF error object and return it.
> >>>> + */
> >>>> + char dummy;
> >>>> + SVN_ERR(svn_io_file_getc(&dummy, file, pool));
> >>>> }
> >>>> +
> >>>> + /* next data chunk */
> >>>> + buf_size -= bytes_read;
> >>>> + buf += bytes_read;
> >>>> + total_read += bytes_read;
> >>>> }
> >>>>
> >>>> + /* buffer limit has been exceeded without finding the EOL */
> >>>> err = svn_io_file_name_get(&name, file, pool);
> >>>> if (err)
> >>>> name = NULL;
> >>>>
> >>>>
> >>>
> >>> Anybody else seeing this?
> >>
> >> Yes, I'm seeing a failure with the same error message, on my Ubuntu
> Linux
> >> system.
> >>
> >> - Julian
> >>
> >>
> >>> I haven't figured out why yet, but r1381800 is causing failures on my
> >>> Windows box, all similar to this:
> >> [...]
> >>> I: svn: E070014: Can't read file
> >>>
> >>
> 'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\repos\db\revs\0\0':
> >>> End of file found
> >> [...]
>
--
*
Join us this October at Subversion Live
2012<http://www.wandisco.com/svn-live-2012>
for two days of best practice SVN training, networking, live demos,
committer meet and greet, and more! Space is limited, so get signed up
today<http://www.wandisco.com/svn-live-2012>
!
*
Re: svn commit: r1381800 - /subversion/trunk/subversion/libsvn_subr/io.c
Posted by Paul Burba <pt...@gmail.com>.
On Fri, Sep 7, 2012 at 3:29 PM, Julian Foad <ju...@btopenworld.com> wrote:
> (Just forwarding this thread to Stefan.)
>
>
> - Julian
>
>
> I (Julian Foad) wrote:
>
>> Paul Burba wrote:
>>
>>> On Thu, Sep 6, 2012 at 7:32 PM, <st...@apache.org> wrote:
>>>> URL: http://svn.apache.org/viewvc?rev=1381800&view=rev
>>>> Log:
>>>> Re-implement svn_io_read_length_line as this is one of the
>>>> most-called functions in SVN. Instead of reading data a byte
>>>> at a time, read 128 byte chunks and scan those.
>> [...]
>>>> ==============================================================================
>>>> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
>>>> +++ subversion/trunk/subversion/libsvn_subr/io.c Thu Sep 6
>>>> @@ -3427,30 +3427,60 @@ svn_error_t *
>>>> svn_io_read_length_line(apr_file_t *file, char *buf, apr_size_t *limit,
>>>> apr_pool_t *pool)
>>>> {
>>>> + /* variables */
>
> p.s. Redundant comment there.
>
>>>> + apr_size_t total_read = 0;
>>>> + svn_boolean_t eof = FALSE;
>>>> const char *name;
>>>> svn_error_t *err;
>>>> - apr_size_t i;
>>>> - char c;
>>>> + apr_size_t buf_size = *limit;
>>>>
>>>> - for (i = 0; i < *limit; i++)
>>>> + while (buf_size > 0)
>>>> {
>>>> - SVN_ERR(svn_io_file_getc(&c, file, pool));
>>>> - /* Note: this error could be APR_EOF, which
>>>> - is totally fine. The caller should be aware of
>>>> - this. */
>>>> -
>>>> - if (c == '\n')
>>>> + /* read a fair chunk of data at once. But don't get too ambitious
>>>> + * as that would result in too much waste. Also make sure we can
>>>> + * put a NUL after the last byte read.
>>>> + */
>>>> + apr_size_t to_read = buf_size < 129 ? buf_size - 1 : 128;
>>>> + apr_size_t bytes_read = 0;
>>>> + char *eol;
>>>> +
>>>> + /* read data block (or just a part of it) */
>>>> + SVN_ERR(svn_io_file_read_full2(file, buf, to_read,
>>>> + &bytes_read, &eof, pool));
>>>> +
>>>> + /* look or a newline char */
>>>> + buf[bytes_read] = 0;
>>>> + eol = strchr(buf, '\n');
>>>> + if (eol)
>>>> {
>>>> - buf[i] = '\0';
>>>> - *limit = i;
>>>> + apr_off_t offset = (eol + 1 - buf) - bytes_read;
Here is the problem: apr_off_t is unsigned and in some cases we are
trying to calculate a negative offset to pass to svn_io_file_seek.
The attached patch fixes this and all tests pass with it, but I'd
appreciate another set of eyes before committing.
[[[
Fix file seek breakage introduced in r1381800.
See also discussion at http://svn.haxx.se/dev/archive-2012-09/0114.shtml
* subversion/libsvn_subr/io.c
(svn_io_read_length_line): Move the read/write file offset to an
absolute offset rather than trying to applying a negative offset to
the current offset, thus preventing the mayhem that comes with
assigning a negative offset value to an unsigned apr_off_t.
]]]
--
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
>>>> + *eol = 0;
>>>> + *limit = total_read + (eol - buf);
>>>> +
>>>> + /* correct the file pointer:
>>>> + * appear as though we just had read the newline char
>>>> + */
>>>> + SVN_ERR(svn_io_file_seek(file, APR_CUR, &offset, pool));
>>>> +
>>>> return SVN_NO_ERROR;
>>>> }
>>>> - else
>>>> + else if (eof)
>>>> {
>>>> - buf[i] = c;
>>>> + /* no EOL found but we hit the end of the file.
>>>> + * Generate a nice EOF error object and return it.
>>>> + */
>>>> + char dummy;
>>>> + SVN_ERR(svn_io_file_getc(&dummy, file, pool));
>>>> }
>>>> +
>>>> + /* next data chunk */
>>>> + buf_size -= bytes_read;
>>>> + buf += bytes_read;
>>>> + total_read += bytes_read;
>>>> }
>>>>
>>>> + /* buffer limit has been exceeded without finding the EOL */
>>>> err = svn_io_file_name_get(&name, file, pool);
>>>> if (err)
>>>> name = NULL;
>>>>
>>>>
>>>
>>> Anybody else seeing this?
>>
>> Yes, I'm seeing a failure with the same error message, on my Ubuntu Linux
>> system.
>>
>> - Julian
>>
>>
>>> I haven't figured out why yet, but r1381800 is causing failures on my
>>> Windows box, all similar to this:
>> [...]
>>> I: svn: E070014: Can't read file
>>>
>> 'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\repos\db\revs\0\0':
>>> End of file found
>> [...]
Re: svn commit: r1381800 - /subversion/trunk/subversion/libsvn_subr/io.c
Posted by Julian Foad <ju...@btopenworld.com>.
(Just forwarding this thread to Stefan.)
- Julian
I (Julian Foad) wrote:
> Paul Burba wrote:
>
>> On Thu, Sep 6, 2012 at 7:32 PM, <st...@apache.org> wrote:
>>> URL: http://svn.apache.org/viewvc?rev=1381800&view=rev
>>> Log:
>>> Re-implement svn_io_read_length_line as this is one of the
>>> most-called functions in SVN. Instead of reading data a byte
>>> at a time, read 128 byte chunks and scan those.
> [...]
>>> ==============================================================================
>>> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
>>> +++ subversion/trunk/subversion/libsvn_subr/io.c Thu Sep 6
>>> @@ -3427,30 +3427,60 @@ svn_error_t *
>>> svn_io_read_length_line(apr_file_t *file, char *buf, apr_size_t *limit,
>>> apr_pool_t *pool)
>>> {
>>> + /* variables */
p.s. Redundant comment there.
>>> + apr_size_t total_read = 0;
>>> + svn_boolean_t eof = FALSE;
>>> const char *name;
>>> svn_error_t *err;
>>> - apr_size_t i;
>>> - char c;
>>> + apr_size_t buf_size = *limit;
>>>
>>> - for (i = 0; i < *limit; i++)
>>> + while (buf_size > 0)
>>> {
>>> - SVN_ERR(svn_io_file_getc(&c, file, pool));
>>> - /* Note: this error could be APR_EOF, which
>>> - is totally fine. The caller should be aware of
>>> - this. */
>>> -
>>> - if (c == '\n')
>>> + /* read a fair chunk of data at once. But don't get too ambitious
>>> + * as that would result in too much waste. Also make sure we can
>>> + * put a NUL after the last byte read.
>>> + */
>>> + apr_size_t to_read = buf_size < 129 ? buf_size - 1 : 128;
>>> + apr_size_t bytes_read = 0;
>>> + char *eol;
>>> +
>>> + /* read data block (or just a part of it) */
>>> + SVN_ERR(svn_io_file_read_full2(file, buf, to_read,
>>> + &bytes_read, &eof, pool));
>>> +
>>> + /* look or a newline char */
>>> + buf[bytes_read] = 0;
>>> + eol = strchr(buf, '\n');
>>> + if (eol)
>>> {
>>> - buf[i] = '\0';
>>> - *limit = i;
>>> + apr_off_t offset = (eol + 1 - buf) - bytes_read;
>>> +
>>> + *eol = 0;
>>> + *limit = total_read + (eol - buf);
>>> +
>>> + /* correct the file pointer:
>>> + * appear as though we just had read the newline char
>>> + */
>>> + SVN_ERR(svn_io_file_seek(file, APR_CUR, &offset, pool));
>>> +
>>> return SVN_NO_ERROR;
>>> }
>>> - else
>>> + else if (eof)
>>> {
>>> - buf[i] = c;
>>> + /* no EOL found but we hit the end of the file.
>>> + * Generate a nice EOF error object and return it.
>>> + */
>>> + char dummy;
>>> + SVN_ERR(svn_io_file_getc(&dummy, file, pool));
>>> }
>>> +
>>> + /* next data chunk */
>>> + buf_size -= bytes_read;
>>> + buf += bytes_read;
>>> + total_read += bytes_read;
>>> }
>>>
>>> + /* buffer limit has been exceeded without finding the EOL */
>>> err = svn_io_file_name_get(&name, file, pool);
>>> if (err)
>>> name = NULL;
>>>
>>>
>>
>> Anybody else seeing this?
>
> Yes, I'm seeing a failure with the same error message, on my Ubuntu Linux
> system.
>
> - Julian
>
>
>> I haven't figured out why yet, but r1381800 is causing failures on my
>> Windows box, all similar to this:
> [...]
>> I: svn: E070014: Can't read file
>>
> 'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\repos\db\revs\0\0':
>> End of file found
> [...]
Re: svn commit: r1381800 - /subversion/trunk/subversion/libsvn_subr/io.c
Posted by Paul Burba <pt...@gmail.com>.
On Fri, Sep 7, 2012 at 6:46 PM, Stefan Fuhrmann
<st...@wandisco.com> wrote:
> On Fri, Sep 7, 2012 at 9:18 PM, Julian Foad <ju...@btopenworld.com>
> wrote:
>>
>> Paul Burba wrote:
>>
>> > On Thu, Sep 6, 2012 at 7:32 PM, <st...@apache.org> wrote:
>> >> URL: http://svn.apache.org/viewvc?rev=1381800&view=rev
>> >> Log:
>> >> Re-implement svn_io_read_length_line as this is one of the
>> >> most-called functions in SVN. Instead of reading data a byte
>> >> at a time, read 128 byte chunks and scan those.
>> [...]
>> >
>> > ==============================================================================
>> >> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
>> >> +++ subversion/trunk/subversion/libsvn_subr/io.c Thu Sep 6 23:32:11
>> >> 2012
>> >> @@ -3427,30 +3427,60 @@ svn_error_t *
>> >> svn_io_read_length_line(apr_file_t *file, char *buf, apr_size_t
>> >> *limit,
>> >> apr_pool_t *pool)
>> >> {
>> >> + /* variables */
>> >> + apr_size_t total_read = 0;
>> >> + svn_boolean_t eof = FALSE;
>> >> const char *name;
>> >> svn_error_t *err;
>> >> - apr_size_t i;
>> >> - char c;
>> >> + apr_size_t buf_size = *limit;
>> >>
>> >> - for (i = 0; i < *limit; i++)
>> >> + while (buf_size > 0)
>> >> {
>> >> - SVN_ERR(svn_io_file_getc(&c, file, pool));
>> >> - /* Note: this error could be APR_EOF, which
>> >> - is totally fine. The caller should be aware of
>> >> - this. */
>> >> -
>> >> - if (c == '\n')
>> >> + /* read a fair chunk of data at once. But don't get too
>> >> ambitious
>> >> + * as that would result in too much waste. Also make sure we
>> >> can
>> >> + * put a NUL after the last byte read.
>> >> + */
>> >> + apr_size_t to_read = buf_size < 129 ? buf_size - 1 : 128;
>> >> + apr_size_t bytes_read = 0;
>> >> + char *eol;
>> >> +
>> >> + /* read data block (or just a part of it) */
>> >> + SVN_ERR(svn_io_file_read_full2(file, buf, to_read,
>> >> + &bytes_read, &eof, pool));
>> >> +
>> >> + /* look or a newline char */
>> >> + buf[bytes_read] = 0;
>> >> + eol = strchr(buf, '\n');
>> >> + if (eol)
>> >> {
>> >> - buf[i] = '\0';
>> >> - *limit = i;
>> >> + apr_off_t offset = (eol + 1 - buf) - bytes_read;
>> >> +
>> >> + *eol = 0;
>> >> + *limit = total_read + (eol - buf);
>> >> +
>> >> + /* correct the file pointer:
>> >> + * appear as though we just had read the newline char
>> >> + */
>> >> + SVN_ERR(svn_io_file_seek(file, APR_CUR, &offset, pool));
>> >> +
>> >> return SVN_NO_ERROR;
>> >> }
>> >> - else
>> >> + else if (eof)
>> >> {
>> >> - buf[i] = c;
>> >> + /* no EOL found but we hit the end of the file.
>> >> + * Generate a nice EOF error object and return it.
>> >> + */
>> >> + char dummy;
>> >> + SVN_ERR(svn_io_file_getc(&dummy, file, pool));
>> >> }
>> >> +
>> >> + /* next data chunk */
>> >> + buf_size -= bytes_read;
>> >> + buf += bytes_read;
>> >> + total_read += bytes_read;
>> >> }
>> >>
>> >> + /* buffer limit has been exceeded without finding the EOL */
>> >> err = svn_io_file_name_get(&name, file, pool);
>> >> if (err)
>> >> name = NULL;
>> >>
>> >>
>> >
>> > Anybody else seeing this?
>
>
> Thanks for the report, Paul!
> Having no buildbots when one needs them is a nuisance.
>
>>
>> Yes, I'm seeing a failure with the same error message, on my Ubuntu Linux
>> system.
>
>
> Thanks for testing. I had no issues on 64 bit Ubuntu.
> => the issue must have been a 32 bit specific one.
>
>>
>>
>> > I haven't figured out why yet, but r1381800 is causing failures on my
>> > Windows box, all similar to this:
>> [...]
>> > I: svn: E070014: Can't read file
>> >
>> > 'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\repos\db\revs\0\0':
>> > End of file found
>> [...]
>
>
> Fixed in r1382196.
Thanks Stefan, I just saw your commit right after I sent my previous
message. My fix was needlessly complex anyway, based on a
misunderstanding I had --somehow I had it in my head that
svn_io_file_seek took size_t rather than apr_off_t. I blame Friday :-\
--
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
> -- Stefan^2.
>
> --
>
> Join us this October at Subversion Live 2012 for two days of best practice
> SVN training, networking, live demos, committer meet and greet, and more!
> Space is limited, so get signed up today!
Re: svn commit: r1381800 - /subversion/trunk/subversion/libsvn_subr/io.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Fri, Sep 7, 2012 at 9:18 PM, Julian Foad <ju...@btopenworld.com>wrote:
> Paul Burba wrote:
>
> > On Thu, Sep 6, 2012 at 7:32 PM, <st...@apache.org> wrote:
> >> URL: http://svn.apache.org/viewvc?rev=1381800&view=rev
> >> Log:
> >> Re-implement svn_io_read_length_line as this is one of the
> >> most-called functions in SVN. Instead of reading data a byte
> >> at a time, read 128 byte chunks and scan those.
> [...]
> >
> ==============================================================================
> >> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> >> +++ subversion/trunk/subversion/libsvn_subr/io.c Thu Sep 6 23:32:11
> 2012
> >> @@ -3427,30 +3427,60 @@ svn_error_t *
> >> svn_io_read_length_line(apr_file_t *file, char *buf, apr_size_t
> *limit,
> >> apr_pool_t *pool)
> >> {
> >> + /* variables */
> >> + apr_size_t total_read = 0;
> >> + svn_boolean_t eof = FALSE;
> >> const char *name;
> >> svn_error_t *err;
> >> - apr_size_t i;
> >> - char c;
> >> + apr_size_t buf_size = *limit;
> >>
> >> - for (i = 0; i < *limit; i++)
> >> + while (buf_size > 0)
> >> {
> >> - SVN_ERR(svn_io_file_getc(&c, file, pool));
> >> - /* Note: this error could be APR_EOF, which
> >> - is totally fine. The caller should be aware of
> >> - this. */
> >> -
> >> - if (c == '\n')
> >> + /* read a fair chunk of data at once. But don't get too
> ambitious
> >> + * as that would result in too much waste. Also make sure we can
> >> + * put a NUL after the last byte read.
> >> + */
> >> + apr_size_t to_read = buf_size < 129 ? buf_size - 1 : 128;
> >> + apr_size_t bytes_read = 0;
> >> + char *eol;
> >> +
> >> + /* read data block (or just a part of it) */
> >> + SVN_ERR(svn_io_file_read_full2(file, buf, to_read,
> >> + &bytes_read, &eof, pool));
> >> +
> >> + /* look or a newline char */
> >> + buf[bytes_read] = 0;
> >> + eol = strchr(buf, '\n');
> >> + if (eol)
> >> {
> >> - buf[i] = '\0';
> >> - *limit = i;
> >> + apr_off_t offset = (eol + 1 - buf) - bytes_read;
> >> +
> >> + *eol = 0;
> >> + *limit = total_read + (eol - buf);
> >> +
> >> + /* correct the file pointer:
> >> + * appear as though we just had read the newline char
> >> + */
> >> + SVN_ERR(svn_io_file_seek(file, APR_CUR, &offset, pool));
> >> +
> >> return SVN_NO_ERROR;
> >> }
> >> - else
> >> + else if (eof)
> >> {
> >> - buf[i] = c;
> >> + /* no EOL found but we hit the end of the file.
> >> + * Generate a nice EOF error object and return it.
> >> + */
> >> + char dummy;
> >> + SVN_ERR(svn_io_file_getc(&dummy, file, pool));
> >> }
> >> +
> >> + /* next data chunk */
> >> + buf_size -= bytes_read;
> >> + buf += bytes_read;
> >> + total_read += bytes_read;
> >> }
> >>
> >> + /* buffer limit has been exceeded without finding the EOL */
> >> err = svn_io_file_name_get(&name, file, pool);
> >> if (err)
> >> name = NULL;
> >>
> >>
> >
> > Anybody else seeing this?
>
Thanks for the report, Paul!
Having no buildbots when one needs them is a nuisance.
> Yes, I'm seeing a failure with the same error message, on my Ubuntu Linux
> system.
>
Thanks for testing. I had no issues on 64 bit Ubuntu.
=> the issue must have been a 32 bit specific one.
>
> > I haven't figured out why yet, but r1381800 is causing failures on my
> > Windows box, all similar to this:
> [...]
> > I: svn: E070014: Can't read file
> >
> 'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\repos\db\revs\0\0':
> > End of file found
> [...]
>
Fixed in r1382196.
-- Stefan^2.
--
*
Join us this October at Subversion Live
2012<http://www.wandisco.com/svn-live-2012>
for two days of best practice SVN training, networking, live demos,
committer meet and greet, and more! Space is limited, so get signed up
today<http://www.wandisco.com/svn-live-2012>
!
*
Re: svn commit: r1381800 - /subversion/trunk/subversion/libsvn_subr/io.c
Posted by Julian Foad <ju...@btopenworld.com>.
Paul Burba wrote:
> On Thu, Sep 6, 2012 at 7:32 PM, <st...@apache.org> wrote:
>> URL: http://svn.apache.org/viewvc?rev=1381800&view=rev
>> Log:
>> Re-implement svn_io_read_length_line as this is one of the
>> most-called functions in SVN. Instead of reading data a byte
>> at a time, read 128 byte chunks and scan those.
[...]
> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
>> +++ subversion/trunk/subversion/libsvn_subr/io.c Thu Sep 6 23:32:11 2012
>> @@ -3427,30 +3427,60 @@ svn_error_t *
>> svn_io_read_length_line(apr_file_t *file, char *buf, apr_size_t *limit,
>> apr_pool_t *pool)
>> {
>> + /* variables */
>> + apr_size_t total_read = 0;
>> + svn_boolean_t eof = FALSE;
>> const char *name;
>> svn_error_t *err;
>> - apr_size_t i;
>> - char c;
>> + apr_size_t buf_size = *limit;
>>
>> - for (i = 0; i < *limit; i++)
>> + while (buf_size > 0)
>> {
>> - SVN_ERR(svn_io_file_getc(&c, file, pool));
>> - /* Note: this error could be APR_EOF, which
>> - is totally fine. The caller should be aware of
>> - this. */
>> -
>> - if (c == '\n')
>> + /* read a fair chunk of data at once. But don't get too ambitious
>> + * as that would result in too much waste. Also make sure we can
>> + * put a NUL after the last byte read.
>> + */
>> + apr_size_t to_read = buf_size < 129 ? buf_size - 1 : 128;
>> + apr_size_t bytes_read = 0;
>> + char *eol;
>> +
>> + /* read data block (or just a part of it) */
>> + SVN_ERR(svn_io_file_read_full2(file, buf, to_read,
>> + &bytes_read, &eof, pool));
>> +
>> + /* look or a newline char */
>> + buf[bytes_read] = 0;
>> + eol = strchr(buf, '\n');
>> + if (eol)
>> {
>> - buf[i] = '\0';
>> - *limit = i;
>> + apr_off_t offset = (eol + 1 - buf) - bytes_read;
>> +
>> + *eol = 0;
>> + *limit = total_read + (eol - buf);
>> +
>> + /* correct the file pointer:
>> + * appear as though we just had read the newline char
>> + */
>> + SVN_ERR(svn_io_file_seek(file, APR_CUR, &offset, pool));
>> +
>> return SVN_NO_ERROR;
>> }
>> - else
>> + else if (eof)
>> {
>> - buf[i] = c;
>> + /* no EOL found but we hit the end of the file.
>> + * Generate a nice EOF error object and return it.
>> + */
>> + char dummy;
>> + SVN_ERR(svn_io_file_getc(&dummy, file, pool));
>> }
>> +
>> + /* next data chunk */
>> + buf_size -= bytes_read;
>> + buf += bytes_read;
>> + total_read += bytes_read;
>> }
>>
>> + /* buffer limit has been exceeded without finding the EOL */
>> err = svn_io_file_name_get(&name, file, pool);
>> if (err)
>> name = NULL;
>>
>>
>
> Anybody else seeing this?
Yes, I'm seeing a failure with the same error message, on my Ubuntu Linux system.
- Julian
> I haven't figured out why yet, but r1381800 is causing failures on my
> Windows box, all similar to this:
[...]
> I: svn: E070014: Can't read file
> 'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\repos\db\revs\0\0':
> End of file found
[...]
Re: svn commit: r1381800 - /subversion/trunk/subversion/libsvn_subr/io.c
Posted by Paul Burba <pt...@gmail.com>.
On Thu, Sep 6, 2012 at 7:32 PM, <st...@apache.org> wrote:
> Author: stefan2
> Date: Thu Sep 6 23:32:11 2012
> New Revision: 1381800
>
> URL: http://svn.apache.org/viewvc?rev=1381800&view=rev
> Log:
> Re-implement svn_io_read_length_line as this is one of the
> most-called functions in SVN. Instead of reading data a byte
> at a time, read 128 byte chunks and scan those.
>
> * subversion/libsvn_subr/io.c
> (svn_io_read_length_line): reimplement
>
> Modified:
> subversion/trunk/subversion/libsvn_subr/io.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/io.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1381800&r1=1381799&r2=1381800&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/io.c Thu Sep 6 23:32:11 2012
> @@ -3427,30 +3427,60 @@ svn_error_t *
> svn_io_read_length_line(apr_file_t *file, char *buf, apr_size_t *limit,
> apr_pool_t *pool)
> {
> + /* variables */
> + apr_size_t total_read = 0;
> + svn_boolean_t eof = FALSE;
> const char *name;
> svn_error_t *err;
> - apr_size_t i;
> - char c;
> + apr_size_t buf_size = *limit;
>
> - for (i = 0; i < *limit; i++)
> + while (buf_size > 0)
> {
> - SVN_ERR(svn_io_file_getc(&c, file, pool));
> - /* Note: this error could be APR_EOF, which
> - is totally fine. The caller should be aware of
> - this. */
> -
> - if (c == '\n')
> + /* read a fair chunk of data at once. But don't get too ambitious
> + * as that would result in too much waste. Also make sure we can
> + * put a NUL after the last byte read.
> + */
> + apr_size_t to_read = buf_size < 129 ? buf_size - 1 : 128;
> + apr_size_t bytes_read = 0;
> + char *eol;
> +
> + /* read data block (or just a part of it) */
> + SVN_ERR(svn_io_file_read_full2(file, buf, to_read,
> + &bytes_read, &eof, pool));
> +
> + /* look or a newline char */
> + buf[bytes_read] = 0;
> + eol = strchr(buf, '\n');
> + if (eol)
> {
> - buf[i] = '\0';
> - *limit = i;
> + apr_off_t offset = (eol + 1 - buf) - bytes_read;
> +
> + *eol = 0;
> + *limit = total_read + (eol - buf);
> +
> + /* correct the file pointer:
> + * appear as though we just had read the newline char
> + */
> + SVN_ERR(svn_io_file_seek(file, APR_CUR, &offset, pool));
> +
> return SVN_NO_ERROR;
> }
> - else
> + else if (eof)
> {
> - buf[i] = c;
> + /* no EOL found but we hit the end of the file.
> + * Generate a nice EOF error object and return it.
> + */
> + char dummy;
> + SVN_ERR(svn_io_file_getc(&dummy, file, pool));
> }
> +
> + /* next data chunk */
> + buf_size -= bytes_read;
> + buf += bytes_read;
> + total_read += bytes_read;
> }
>
> + /* buffer limit has been exceeded without finding the EOL */
> err = svn_io_file_name_get(&name, file, pool);
> if (err)
> name = NULL;
>
>
Anybody else seeing this?
I haven't figured out why yet, but r1381800 is causing failures on my
Windows box, all similar to this:
C:\SVN\src-trunk>win-tests.py -d -c --test=basic#1 --log-to-stdout
--log-level=DEBUG
Testing Debug configuration on local repository.
START: basic_tests.py
I: CMD: svnadmin.exe create svn-test-work\local_tmp\repos
--bdb-txn-nosync --fs-type=fsfs
I: <TIME = 1.990000>
I: CMD: svn.exe import -m "Log message for revision 1."
svn-test-work\local_tmp\greekfiles
file:///C:/SVN/src-trunk/Debug/subversion/tests/cmdline/svn-test-work
/local_tmp/repos --config-dir
C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password rayjandom --no-auth-cache --username jra
ndom
I: CMD: C:\SVN\src-trunk\Debug\subversion\svn\svn.exe import -m "Log
message for revision 1." svn-test-work\local_tmp\greekfiles
file:///C:/SVN/src-trunk/Debug/
subversion/tests/cmdline/svn-test-work/local_tmp/repos --config-dir
C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password ra
yjandom --no-auth-cache --username jrandom exited with 1
I: <TIME = 4.514000>
I: ..\..\..\subversion\svn\import-cmd.c:128: (apr_err=70014)
I: ..\..\..\subversion\libsvn_client\commit.c:976: (apr_err=70014)
I: ..\..\..\subversion\libsvn_client\commit.c:712: (apr_err=70014)
I: ..\..\..\subversion\libsvn_client\commit.c:442: (apr_err=70014)
I: ..\..\..\subversion\libsvn_client\commit.c:535: (apr_err=70014)
I: ..\..\..\subversion\libsvn_repos\commit.c:348: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs\fs-loader.c:1123: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\tree.c:1827: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\tree.c:675: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\dag.c:1147: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\dag.c:315: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5340: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5291: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5136: (apr_err=70014)
I: ..\..\..\subversion\libsvn_subr\stream.c:143: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:4905: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:4839: (apr_err=70014)
I: ..\..\..\subversion\libsvn_subr\io.c:3215: (apr_err=70014)
I: svn: E070014: Can't read file
'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\repos\db\revs\0\0':
End of file found
W: ..\..\..\subversion\svn\import-cmd.c:128: (apr_err=70014)
W: ..\..\..\subversion\libsvn_client\commit.c:976: (apr_err=70014)
W: ..\..\..\subversion\libsvn_client\commit.c:712: (apr_err=70014)
W: ..\..\..\subversion\libsvn_client\commit.c:442: (apr_err=70014)
W: ..\..\..\subversion\libsvn_client\commit.c:535: (apr_err=70014)
W: ..\..\..\subversion\libsvn_repos\commit.c:348: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs\fs-loader.c:1123: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\tree.c:1827: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\tree.c:675: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\dag.c:1147: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\dag.c:315: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5340: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5291: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5136: (apr_err=70014)
W: ..\..\..\subversion\libsvn_subr\stream.c:143: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:4905: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:4839: (apr_err=70014)
W: ..\..\..\subversion\libsvn_subr\io.c:3215: (apr_err=70014)
W: svn: E070014: Can't read file
'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\repos\db\revs\0\0':
End of file found
END: basic_tests.py
ELAPSED: basic_tests.py 0:00:06.609000
--
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
Re: svn commit: r1381800 - /subversion/trunk/subversion/libsvn_subr/io.c
Posted by Paul Burba <pt...@gmail.com>.
On Thu, Sep 6, 2012 at 7:32 PM, <st...@apache.org> wrote:
> Author: stefan2
> Date: Thu Sep 6 23:32:11 2012
> New Revision: 1381800
>
> URL: http://svn.apache.org/viewvc?rev=1381800&view=rev
> Log:
> Re-implement svn_io_read_length_line as this is one of the
> most-called functions in SVN. Instead of reading data a byte
> at a time, read 128 byte chunks and scan those.
>
> * subversion/libsvn_subr/io.c
> (svn_io_read_length_line): reimplement
>
> Modified:
> subversion/trunk/subversion/libsvn_subr/io.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/io.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=1381800&r1=1381799&r2=1381800&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/io.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/io.c Thu Sep 6 23:32:11 2012
> @@ -3427,30 +3427,60 @@ svn_error_t *
> svn_io_read_length_line(apr_file_t *file, char *buf, apr_size_t *limit,
> apr_pool_t *pool)
> {
> + /* variables */
> + apr_size_t total_read = 0;
> + svn_boolean_t eof = FALSE;
> const char *name;
> svn_error_t *err;
> - apr_size_t i;
> - char c;
> + apr_size_t buf_size = *limit;
>
> - for (i = 0; i < *limit; i++)
> + while (buf_size > 0)
> {
> - SVN_ERR(svn_io_file_getc(&c, file, pool));
> - /* Note: this error could be APR_EOF, which
> - is totally fine. The caller should be aware of
> - this. */
> -
> - if (c == '\n')
> + /* read a fair chunk of data at once. But don't get too ambitious
> + * as that would result in too much waste. Also make sure we can
> + * put a NUL after the last byte read.
> + */
> + apr_size_t to_read = buf_size < 129 ? buf_size - 1 : 128;
> + apr_size_t bytes_read = 0;
> + char *eol;
> +
> + /* read data block (or just a part of it) */
> + SVN_ERR(svn_io_file_read_full2(file, buf, to_read,
> + &bytes_read, &eof, pool));
> +
> + /* look or a newline char */
> + buf[bytes_read] = 0;
> + eol = strchr(buf, '\n');
> + if (eol)
> {
> - buf[i] = '\0';
> - *limit = i;
> + apr_off_t offset = (eol + 1 - buf) - bytes_read;
> +
> + *eol = 0;
> + *limit = total_read + (eol - buf);
> +
> + /* correct the file pointer:
> + * appear as though we just had read the newline char
> + */
> + SVN_ERR(svn_io_file_seek(file, APR_CUR, &offset, pool));
> +
> return SVN_NO_ERROR;
> }
> - else
> + else if (eof)
> {
> - buf[i] = c;
> + /* no EOL found but we hit the end of the file.
> + * Generate a nice EOF error object and return it.
> + */
> + char dummy;
> + SVN_ERR(svn_io_file_getc(&dummy, file, pool));
> }
> +
> + /* next data chunk */
> + buf_size -= bytes_read;
> + buf += bytes_read;
> + total_read += bytes_read;
> }
>
> + /* buffer limit has been exceeded without finding the EOL */
> err = svn_io_file_name_get(&name, file, pool);
> if (err)
> name = NULL;
>
>
Anybody else seeing this?
I haven't figured out why yet, but r1381800 is causing failures on my
Windows box, all similar to this:
C:\SVN\src-trunk>win-tests.py -d -c --test=basic#1 --log-to-stdout
--log-level=DEBUG
Testing Debug configuration on local repository.
START: basic_tests.py
I: CMD: svnadmin.exe create svn-test-work\local_tmp\repos
--bdb-txn-nosync --fs-type=fsfs
I: <TIME = 1.990000>
I: CMD: svn.exe import -m "Log message for revision 1."
svn-test-work\local_tmp\greekfiles
file:///C:/SVN/src-trunk/Debug/subversion/tests/cmdline/svn-test-work
/local_tmp/repos --config-dir
C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password rayjandom --no-auth-cache --username jra
ndom
I: CMD: C:\SVN\src-trunk\Debug\subversion\svn\svn.exe import -m "Log
message for revision 1." svn-test-work\local_tmp\greekfiles
file:///C:/SVN/src-trunk/Debug/
subversion/tests/cmdline/svn-test-work/local_tmp/repos --config-dir
C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\config
--password ra
yjandom --no-auth-cache --username jrandom exited with 1
I: <TIME = 4.514000>
I: ..\..\..\subversion\svn\import-cmd.c:128: (apr_err=70014)
I: ..\..\..\subversion\libsvn_client\commit.c:976: (apr_err=70014)
I: ..\..\..\subversion\libsvn_client\commit.c:712: (apr_err=70014)
I: ..\..\..\subversion\libsvn_client\commit.c:442: (apr_err=70014)
I: ..\..\..\subversion\libsvn_client\commit.c:535: (apr_err=70014)
I: ..\..\..\subversion\libsvn_repos\commit.c:348: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs\fs-loader.c:1123: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\tree.c:1827: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\tree.c:675: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\dag.c:1147: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\dag.c:315: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5340: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5291: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5136: (apr_err=70014)
I: ..\..\..\subversion\libsvn_subr\stream.c:143: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:4905: (apr_err=70014)
I: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:4839: (apr_err=70014)
I: ..\..\..\subversion\libsvn_subr\io.c:3215: (apr_err=70014)
I: svn: E070014: Can't read file
'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\repos\db\revs\0\0':
End of file found
W: ..\..\..\subversion\svn\import-cmd.c:128: (apr_err=70014)
W: ..\..\..\subversion\libsvn_client\commit.c:976: (apr_err=70014)
W: ..\..\..\subversion\libsvn_client\commit.c:712: (apr_err=70014)
W: ..\..\..\subversion\libsvn_client\commit.c:442: (apr_err=70014)
W: ..\..\..\subversion\libsvn_client\commit.c:535: (apr_err=70014)
W: ..\..\..\subversion\libsvn_repos\commit.c:348: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs\fs-loader.c:1123: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\tree.c:1827: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\tree.c:675: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\dag.c:1147: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\dag.c:315: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5340: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5291: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:5136: (apr_err=70014)
W: ..\..\..\subversion\libsvn_subr\stream.c:143: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:4905: (apr_err=70014)
W: ..\..\..\subversion\libsvn_fs_fs\fs_fs.c:4839: (apr_err=70014)
W: ..\..\..\subversion\libsvn_subr\io.c:3215: (apr_err=70014)
W: svn: E070014: Can't read file
'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\local_tmp\repos\db\revs\0\0':
End of file found
END: basic_tests.py
ELAPSED: basic_tests.py 0:00:06.609000
--
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
Re: svn commit: r1381800 - /subversion/trunk/subversion/libsvn_subr/io.c
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Fri, Sep 7, 2012 at 10:11 AM, Ivan Zhakov <iv...@visualsvn.com> wrote:
> On Fri, Sep 7, 2012 at 3:32 AM, <st...@apache.org> wrote:
> > Author: stefan2
> > Date: Thu Sep 6 23:32:11 2012
> > New Revision: 1381800
> >
> > URL: http://svn.apache.org/viewvc?rev=1381800&view=rev
> > Log:
> > Re-implement svn_io_read_length_line as this is one of the
> > most-called functions in SVN. Instead of reading data a byte
> > at a time, read 128 byte chunks and scan those.
> >
> Hi Stefan,
>
> Why do not use APR internal buffering? I meant APR_BUFFERED flag.
>
I Ivan,
The flag is set. But reading a single byte invokes a
total of 6 functions - each doing its own thing.
'SVN log $tsvn/trunk -v' will call svn_io_read_length_line
316,418 times. In turn, it would call svn_io_file_getc
8,623,854 times - consuming >20% of the CPU time.
With my patch, the fraction is now below 2%.
-- Stefan^2.
--
*
Join us this October at Subversion Live
2012<http://www.wandisco.com/svn-live-2012>
for two days of best practice SVN training, networking, live demos,
committer meet and greet, and more! Space is limited, so get signed up
today<http://www.wandisco.com/svn-live-2012>
!
*
Re: svn commit: r1381800 - /subversion/trunk/subversion/libsvn_subr/io.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Fri, Sep 7, 2012 at 3:32 AM, <st...@apache.org> wrote:
> Author: stefan2
> Date: Thu Sep 6 23:32:11 2012
> New Revision: 1381800
>
> URL: http://svn.apache.org/viewvc?rev=1381800&view=rev
> Log:
> Re-implement svn_io_read_length_line as this is one of the
> most-called functions in SVN. Instead of reading data a byte
> at a time, read 128 byte chunks and scan those.
>
Hi Stefan,
Why do not use APR internal buffering? I meant APR_BUFFERED flag.
--
Ivan Zhakov