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.
On Thu, Apr 22, 2010 at 12:44 PM, Andrei Alexandrescu <[email protected]>wrote: > 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. > > 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. > > > Andrei > > > On 04/19/2010 11:32 AM, Steve Schveighoffer wrote: > >> This is a tricky situation. I don't think assumeSafeAppend is the right >> call. Imagine something like this: >> >> char[] data = new char[50]; >> data[40..45] = "hello"; >> >> f.readln(data[0..40]); >> >> now, if the line extends past 40 characters, it will overwrite >> data[40..45]. >> >> What I think the safe thing to do is to not use appending until the >> buffer is exhausted. However, even in that case, I don't think append is >> the right move. Appending one char at a time, while maybe faster when >> the buffer can append in place, is still extremely slow compared to just >> setting a char in an array. A better solution is to simply increment the >> length of the buffer when needed. A handy function from the runtime >> would extend an array to the full capacity it could contain. >> >> A quick (uncompiled) version of the relevant part: >> >> uint used = 0; >> char[4] encbuf; >> uint enclen; >> for (int c; (c = FGETWC(fp)) != -1; ) >> { >> if ((c & ~0x7F) == 0) >> { >> enclen = 1; >> encbuf[0] = cast(char)c; >> } >> else >> { >> enclen = std.utf.encode(encbuf, cast(dchar)c); >> } >> if(used + enclen > buf.length) >> { >> /* this should be a runtime function */ >> buf.length = used + enclen; >> buf.length = buf.capacity; >> } >> buf[used..used+enclen] = encbuf[0..enclen]; >> if (c == terminator) >> break; >> } >> if (ferror(fps)) >> StdioException(); >> return used; >> >> >> This is where a D-specific buffering solution would be extremely useful. >> Instead of using FGETC, you could simply gain access to the buffer, and >> search for the terminator using std.algorithm. Tango uses this kind of >> thing. >> >> -Steve >> >> >> *From:* David Simcha <[email protected]> >> *To:* Discuss the phobos library for D <[email protected]> >> *Sent:* Mon, April 19, 2010 11:30:43 AM >> *Subject:* [phobos] readlnImpl (Again) >> >> I've begun to realize that Steve's array stomping patch breaks some >> assumptions that std.stdio.readlnImpl made about how array appending >> works, leading to it being slower than molasses again. Should I just >> put some assumeSafeAppend() calls in there to make it behave as it >> used to, even though the old behavior was unsafe, or do we want to >> completely redesign this code now that we want to guarantee no >> stomping? >> >> >> >> >> _______________________________________________ >> phobos mailing list >> [email protected] >> http://lists.puremagic.com/mailman/listinfo/phobos >> > _______________________________________________ > phobos mailing list > [email protected] > http://lists.puremagic.com/mailman/listinfo/phobos >
_______________________________________________ phobos mailing list [email protected] http://lists.puremagic.com/mailman/listinfo/phobos
