Nim icon indicating copy to clipboard operation
Nim copied to clipboard

Extremely slow slice assignment implementation

Open arnetheduck opened this issue 2 years ago • 3 comments

What happened?

https://github.com/nim-lang/Nim/blob/a4d63ca8ba012d4a65a66b655e98d7ea79c313db/lib/system.nim#L2619 - the current slice assignment is implemented using a for loop over the indices.

In particular, this means that for each entry in the loop, a range check is done - the observable outcome of this implementation is that the range check error can happen after some entries have been copied meaning that the compiler is not free to vectorize the loop using larger data types, and instead must interleave byte copying and range checking - compared to copyMem, this orders of magnitude slower.

Nim Version

a4d63ca8ba012d4a65a66b655e98d7ea79c313db

Current Standard Output Logs

No response

Expected Standard Output Logs

No response

Possible Solution

No response

Additional Information

No response

arnetheduck avatar Aug 15 '22 14:08 arnetheduck

Related, why isn't the slice operation (s[a..b]) CoW ?

AmjadHD avatar Sep 07 '22 10:09 AmjadHD

Related, why isn't the slice operation (s[a..b]) CoW ?

The technical reason: because strings are mutable, and implemented as (essentially) a pointer to a dynamically allocated chunk of memory. If you move that pointer to somewhere in another string, then A: writes to one string will affect writes to the other, and B: the garbage collector won't be able to easily collect the string.

The nontechnical reason (that I don't necessarily agree with): CoW behavior is rarely worth the added computational cost, and can lead to odd bugs. For example, taking a small slice of a large string might keep that large string in memory, when it would otherwise be garbage-collected. I believe at one time Java had CoW strings, and later moved away from that behavior due to this.

Varriount avatar Sep 07 '22 19:09 Varriount

compared to copyMem, this orders of magnitude slower.

Is there a technical reason for why copyMem is not used here ?

AmjadHD avatar Sep 19 '22 15:09 AmjadHD