Skip to content

Conversation

ringabout
Copy link
Member

@ringabout ringabout commented Jun 30, 2025

fixes #25007

proc setLengthSeqUninit(s: PGenericSeq, typ: PNimType, newLen: int, isTrivial: bool): PGenericSeq {.
    compilerRtl.} =

In this added function, only the line zeroMem(dataPointer(result, elemAlign, elemSize, newLen), (result.len-%newLen) *% elemSize) is removed from proc setLengthSeqV2 when enlarging a sequence.

JS and VM versions simply use setLen.

@Araq Araq merged commit 611b8bb into devel Jul 14, 2025
18 checks passed
@Araq Araq deleted the pr_add_refc branch July 14, 2025 21:20
Copy link
Contributor

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 611b8bb

Hint: mm: orc; opt: speed; options: -d:release
182867 lines; 8.794s; 659.902MiB peakmem

@ZoomRmc
Copy link
Contributor

ZoomRmc commented Sep 21, 2025

Looks like when setLengthSeqUninit called for s == nil and newLen != 0 it calls basic newSeq which relies on zeroing newObj for initialization.

There's a separate newSeq implementation for each GC, so it seems like the easiest path is to modify them to conditionally call newObj or newObjNoInit (which is not defined for all the GCs).

ZoomRmc added a commit to ZoomRmc/Nim that referenced this pull request Sep 21, 2025
Continuation of nim-lang#25180. This one refactors the sequence routines.

Preparation for extending with new routines.

Mostly removes repeating code to simplify debugging.

Removes:
 - `incrSeq` and `incrSeqV2` superseded by `incrSeqV3`,
 - `setLengthSeq` superseded by `setLengthSeqV2`

Note comment on line 338, acknowledging that implementation of
`setLenUninit` from nim-lang#25022 does zero the new memory in this branch,
having been copied from `setLengthSeqV2`. This PR does not fix this.
ZoomRmc added a commit to ZoomRmc/Nim that referenced this pull request Sep 21, 2025
Continuation of nim-lang#25180. This one refactors the sequence routines.

Preparation for extending with new routines.

Mostly removes repeating code to simplify debugging.

Removes:
 - `incrSeqV2` superseded by `incrSeqV3`,
 - `setLengthSeq` superseded by `setLengthSeqV2`

Note comment on line 338, acknowledging that implementation of
`setLenUninit` from nim-lang#25022 does zero the new memory in this branch,
having been copied from `setLengthSeqV2`. This PR does not fix this.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

setLenUninit missing for --mm:refc
3 participants