On 23 October 2015, Ivan Zhakov wrote:
> On 23 October 2015, Julian Foad wrote:
>> 1. The doc string should explain len_hint in the caller's terms. The
>> doc strings also differ in other unnecessary respects. I propose the
>> attached documentation patch.
>>
> I agree. Patch look great!
Thanks.
Actually, one part of the doc string I suggested is not good:
+ * The present implementation:
+ * - uses an effective hint of 64 bytes when a lower value or zero is
+ * specified;
+ * - uses minimal time and space when the actual length is less than or
+ * equal to the effective hint;
+ * - uses around 1.5x to 2x the minimal time and space when the actual
+ * length is greater than the effective hint.
That last comment, "around 1.5x to 2x the minimal time and space", is
misleading.
On thinking more carefully, I think in fact it adds a data copying
time overhead from 1x to 2x the actual length (on top of the necessary
1x copying to get the data into result_pool), and allocates additional
memory space from 2x to 3x the actual length (on top of the necessary
space to hold the result), because of the size doubling algorithm we
use.
It's probably better if we say something less specific in the doc
string, and perhaps add the above notes in the implementation file.
People who care that much can read the source.
Committed revision 1710201.
>> 2. The _string and _stringbuf function declarations still differ in
>> that only svn_string_from_stream2() takes a scratch pool. [...]
>>
>> In the absence of any other argument, I would accept that argument and
>> remove the scratch pool.
>>
> I don't have opinion on this, so feel free to remove scratch_pool
> argument if you think that it will be useful.
Consistency is good.
Committed revision 1710202.
- Julian
Received on 2015-10-23 14:58:46 CEST