Bug 18899 – destroy is inefficient for small structs

Status
NEW
Severity
enhancement
Priority
P4
Component
druntime
Product
D
Version
D2
Platform
All
OS
All
Creation time
2018-05-23T15:41:11Z
Last change time
2024-12-07T13:38:20Z
Assigned to
No Owner
Creator
Steven Schveighoffer
Moved to GitHub: dmd#17365 →

Comments

Comment #0 by schveiguy — 2018-05-23T15:41:11Z
When destroy is called on a small struct, it runs code like this: shared static immutable T init = T.init; _destructRecurse(obj); () @trusted { auto dest = (cast(ubyte*) &obj)[0 .. T.sizeof]; auto src = (cast(ubyte*) &init)[0 .. T.sizeof]; dest[] = src[]; } (); Which is WAY overkill for a struct like: struct S { int x; } Using obj = T.init should be done for cases where it's proven to be proper. In other words, no postblit (or disabled postblit). It used to be that this function used typeid, and the initializer within. One of the speedups is if the type is all 0's, then buf[] = 0 can be used. Not sure if there's a mechanism to tell if a type is all zeros, but if it can be done, that would be faster. In addition, maybe using the ubyte array is more efficient in some cases, depending on the size. But I'm not sure.
Comment #1 by turkeyman — 2018-05-23T23:27:09Z
So, memcpy... as opposed to primitive assignment?
Comment #2 by slavo5150 — 2018-05-24T01:10:36Z
> dest[] = src[] I'm pretty sure this gets lowered to memcpy by the compiler, but I haven't verified yet.
Comment #3 by slavo5150 — 2018-05-24T01:31:43Z
Yes, `dest[] = src[]` gets lowered to _d_arraycopy, which ultimately calls memcpy View ASM here: https://run.dlang.io/is/izCLp0 _d_arraycopy implementation here: https://github.com/dlang/druntime/blob/2db828bd4f21807254b770b3ec304f14596a9805/src/rt/arraycat.d#L22-L29
Comment #4 by turkeyman — 2018-05-24T01:56:14Z
Yes, that's what I'm saying :) ... you're not happy with memcpy, do want element-copy?
Comment #5 by schveiguy — 2018-05-24T09:12:23Z
(In reply to Manu from comment #4) > Yes, that's what I'm saying :) ... you're not happy with memcpy, do want > element-copy? It's more that I want to copy the one int that is in the struct than call an opaque function, no matter how great that function is implemented.
Comment #6 by turkeyman — 2018-05-24T18:37:16Z
memcpy should be an intrinsic, which is implemented using magic...
Comment #7 by schveiguy — 2018-05-24T19:15:39Z
I'm not sure that it is. But we aren't calling memcpy anyway, we are calling _d_arraycopy, not inlined. See the generated AST from Mike's example.
Comment #8 by schveiguy — 2018-05-24T19:16:08Z
(In reply to Steven Schveighoffer from comment #7) > See the generated AST ...generated *assembly*
Comment #9 by turkeyman — 2018-05-24T19:26:35Z
True. Anyway, I'm just playing devils advocate. I agree, it should do an element copy for small structs.
Comment #10 by nick — 2018-06-09T11:09:36Z
For the record, Andrei wants to solve this by not blitting init for primitive types: https://github.com/dlang/druntime/pull/2126 > memcpy should be an intrinsic Would that allow unrolling loops when the length is known at compile-time? An interim improvement might be to have an array version of std.algorithm.copy in druntime (which phobos then uses internally).
Comment #11 by robert.schadek — 2024-12-07T13:38:20Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/17365 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB