std.array.join GC optimisation #2746
Conversation
| auto arr = ["Здравствуй", "Мир", "Unicode"].to!(T[]); | ||
| foreach (S; TypeTuple!(wchar,dchar)) | ||
| { | ||
| auto jarr = arr.join(to!S('⧑')); |
There was a problem hiding this comment.
Should be to!S(' ') not to!S('⧑')?
There was a problem hiding this comment.
This is special test for characters greater then 255.
Test case for ' ' is above.
There was a problem hiding this comment.
Oh sorry... it's a font bug in my browser, it looked like a space with a different ending single quote, but actually it's a unicode character between two normal single quotes that didn't get rendered properly by my browser. Sorry for the noise.
There was a problem hiding this comment.
OTOH, you might want to consider using \uxxxx notation instead, in case someone doesn't have the right fonts installed to display that particular character.
|
Also, please provide some benchmark results so that we are sure this change actually improves things, especially since the new code scans |
|
|
Oh I see. It wasn't very clear from the PR title. :-) |
|
But isn't |
struct Appender(A)
{
private struct Data
{
size_t capacity;
Unqual!T[] arr;
bool canExtend = false;
}
private Data* _data; // <------------------------------- |
|
Hmph. Apparently that was a later change to make appender have reference semantics. :-( Alright then, let's merge this once you fix the other stuff. |
update PR
|
Done. |
|
Thanks! |
|
Auto-merge toggled on |
std.array.join GC optimisation
| foreach(r; ror) | ||
| { | ||
| foreach(e; sepArr) | ||
| result[len++] = e; |
There was a problem hiding this comment.
Ehm ... what if T has destructors? uninitialzedArray looks dangerous.
emplace probably is a better fit to put something into the right place in memory.
|
Fix in PR #2747 |
| return ret - sepLength; | ||
| } | ||
|
|
||
| private U trustedCast(U, V)(V v) @trusted pure nothrow { return cast(U) v; } |
There was a problem hiding this comment.
I doubt that creating unsafe trusted functions at module scope is a good idea. In fact, I personally think it's quite an awful one. If I now read the std.array source code, I have to audit the whole module for possibly illegal uses of it. If you need it in two places, make two (probably static, to avoid allocating a closure) nested helper functions. Also helps to save on template instantiations, although that's hardly significant.
|
There are quite a few code quality issues with this PR. Please address them too. |
|
see #2747 |
No description provided.