Tobias Ringstrom wrote:
> Branko Čibej wrote:
>
>> bliss@tigris.org wrote:
>>
>>> * subversion/include/svn_types.h
>>> (SVN_STREAM_CHUNK_SIZE): Reduce from 100 kiB to 8 kiB.
>>> (SVN_MAX_OBJECT_SIZE): Remove strange FIXME.
>>
>>
>> I object to this last bit. It is not a "strange FIXME", it's a question
>> that needs to be answered before doing anything about it. Just removing
>> the comment doesn't answer the question, and you didn't even try to
>> analyse the pros and cons of making max-window-size the same as
>> stream-chunk-size.
>
>
> (I think you meant max-object-size, not max-window-size.)
Yes, sorry.
> How about removing SVN_MAX_OBJECT_SIZE and replacing it with something
> that makes sense? I don't see how you can have a compile time limit
> on what is a sensible maximum object size. An object can be anything.
There is a maximum contiguous memory block that can be handled by a
particular architecture. Usually it is maxsize_t). We use this constant
to check if the contents will "fit" in memory before trying to read them
from the database.
> The limit is currently used for only one thing in the code, namely to
> sanity check the size of a rep in the fs code. Renaming it to
> SVN_MAX_REP_SIZE would make it easier to assign a reasonable size to it.
It isn't exactly the same as a maximum rep size. Notice that we only
check this in svn_fs__rep_contents, which reads the whole rep into
memory. The streamy read functions don't have to check for that maximum.
And that's what the question in the FIXME was about: Can we reasonably
limit this size to, e.g., 100k or not?
> Does this sound OK, and do you have a suggestion for a good max rep size?
If I had, I'd have defined it and not left questions in the code. :-)
I do have a question for you, though: Did you chech what happens with
streamy reads (and writes) from the DB now that you've changed the
stream chunk size? Do we now store large fulltexts in 8kiB chunks
instead of 100kiB chunks?
Id you don't know, then may I suggest you revert the patch until you
_do_ know?
>>> (SVN_DELTA_WINDOW_SIZE): New define to use for the svndiff window
>>> size instead of SVN_STREAM_CHUNK_SIZE.
>>> (svn_txdelta, svn_txdelta_next_window): Use SVN_DELTA_WINDOW_SIZE.
>>
>>
>> I believe you missed at least fs-test. Your change modifies the
>> semantics of the file integrity tests.
>
> * subversion/libsvn_delta/text_delta.c
>
> I wouldn't commit it if not all tests had passed. What do you mean?
The large-file-integrity tests are specifically designed to generate
files that use up more than one delta window. Now that you've split
stream size and delta window size, I believe these tests no longer
excersize the delta window boundary.
--
Brane Čibej <brane_at_xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Apr 4 02:35:03 2004