I was already using std.array.Appender since I fixed this the first time. I added the assumeSafeAppend calls in my own source tree, but I didn't commit it both because I wanted input on whether we still want to allow stomping and because for some reason its still horribly slow. Whether Appender still does what it's supposed to is a good question.<br>
<br><div class="gmail_quote">On Thu, Apr 22, 2010 at 12:44 PM, Andrei Alexandrescu <span dir="ltr"><<a href="mailto:andrei@erdani.com">andrei@erdani.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
Guys, I'll let you make sure this is taken care of; I'm busy with the final touches for TDPL. Once I let it out of my hand, that will really be it.<br>
<br>
Just a brief thought - I hope std.array.Appender still works with the new allocation regime. If it does, maybe it could be put to use for readln.<br>
<br>
<br>
Andrei<div><div></div><div class="h5"><br>
<br>
On 04/19/2010 11:32 AM, Steve Schveighoffer wrote:<br>
</div></div><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;"><div><div></div><div class="h5">
This is a tricky situation. I don't think assumeSafeAppend is the right<br>
call. Imagine something like this:<br>
<br>
char[] data = new char[50];<br>
data[40..45] = "hello";<br>
<br>
f.readln(data[0..40]);<br>
<br>
now, if the line extends past 40 characters, it will overwrite data[40..45].<br>
<br>
What I think the safe thing to do is to not use appending until the<br>
buffer is exhausted. However, even in that case, I don't think append is<br>
the right move. Appending one char at a time, while maybe faster when<br>
the buffer can append in place, is still extremely slow compared to just<br>
setting a char in an array. A better solution is to simply increment the<br>
length of the buffer when needed. A handy function from the runtime<br>
would extend an array to the full capacity it could contain.<br>
<br>
A quick (uncompiled) version of the relevant part:<br>
<br>
uint used = 0;<br>
char[4] encbuf;<br>
uint enclen;<br>
for (int c; (c = FGETWC(fp)) != -1; )<br>
{<br>
if ((c & ~0x7F) == 0)<br>
{<br>
enclen = 1;<br>
encbuf[0] = cast(char)c;<br>
}<br>
else<br>
{<br>
enclen = std.utf.encode(encbuf, cast(dchar)c);<br>
}<br>
if(used + enclen > buf.length)<br>
{<br>
/* this should be a runtime function */<br>
buf.length = used + enclen;<br>
buf.length = buf.capacity;<br>
}<br>
buf[used..used+enclen] = encbuf[0..enclen];<br>
if (c == terminator)<br>
break;<br>
}<br>
if (ferror(fps))<br>
StdioException();<br>
return used;<br>
<br>
<br>
This is where a D-specific buffering solution would be extremely useful.<br>
Instead of using FGETC, you could simply gain access to the buffer, and<br>
search for the terminator using std.algorithm. Tango uses this kind of<br>
thing.<br>
<br>
-Steve<br>
<br>
<br>
*From:* David Simcha <<a href="mailto:dsimcha@gmail.com" target="_blank">dsimcha@gmail.com</a>><br>
*To:* Discuss the phobos library for D <<a href="mailto:phobos@puremagic.com" target="_blank">phobos@puremagic.com</a>><br>
*Sent:* Mon, April 19, 2010 11:30:43 AM<br>
*Subject:* [phobos] readlnImpl (Again)<br>
<br>
I've begun to realize that Steve's array stomping patch breaks some<br>
assumptions that std.stdio.readlnImpl made about how array appending<br>
works, leading to it being slower than molasses again. Should I just<br>
put some assumeSafeAppend() calls in there to make it behave as it<br>
used to, even though the old behavior was unsafe, or do we want to<br>
completely redesign this code now that we want to guarantee no stomping?<br>
<br>
<br>
<br>
<br></div></div>
_______________________________________________<br>
phobos mailing list<br>
<a href="mailto:phobos@puremagic.com" target="_blank">phobos@puremagic.com</a><br>
<a href="http://lists.puremagic.com/mailman/listinfo/phobos" target="_blank">http://lists.puremagic.com/mailman/listinfo/phobos</a><br>
</blockquote>
_______________________________________________<br>
phobos mailing list<br>
<a href="mailto:phobos@puremagic.com" target="_blank">phobos@puremagic.com</a><br>
<a href="http://lists.puremagic.com/mailman/listinfo/phobos" target="_blank">http://lists.puremagic.com/mailman/listinfo/phobos</a><br>
</blockquote></div><br>