Skip to content

Commit aa68c89

Browse files
committed
std: sysstr refactor
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.
1 parent 3f48576 commit aa68c89

File tree

1 file changed

+54
-105
lines changed

1 file changed

+54
-105
lines changed

lib/system/sysstr.nim

Lines changed: 54 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -252,14 +252,6 @@ proc incrSeq(seq: PGenericSeq, elemSize, elemAlign: int): PGenericSeq {.compiler
252252
result.reserved = r
253253
inc(result.len)
254254

255-
proc incrSeqV2(seq: PGenericSeq, elemSize, elemAlign: int): PGenericSeq {.compilerproc.} =
256-
# incrSeq version 2
257-
result = seq
258-
if result.len >= result.space:
259-
let r = resize(result.space)
260-
result = cast[PGenericSeq](growObj(result, align(GenericSeqSize, elemAlign) + elemSize * r))
261-
result.reserved = r
262-
263255
proc incrSeqV3(s: PGenericSeq, typ: PNimType): PGenericSeq {.compilerproc.} =
264256
if s == nil:
265257
result = cast[PGenericSeq](newSeq(typ, 1))
@@ -274,112 +266,68 @@ proc incrSeqV3(s: PGenericSeq, typ: PNimType): PGenericSeq {.compilerproc.} =
274266
# since we steal the content from 's', it's crucial to set s's len to 0.
275267
s.len = 0
276268

277-
proc setLengthSeq(seq: PGenericSeq, elemSize, elemAlign, newLen: int): PGenericSeq {.
278-
compilerRtl, inl.} =
279-
result = seq
280-
if result.space < newLen:
281-
let r = max(resize(result.space), newLen)
282-
result = cast[PGenericSeq](growObj(result, align(GenericSeqSize, elemAlign) + elemSize * r))
283-
result.reserved = r
284-
elif newLen < result.len:
285-
# we need to decref here, otherwise the GC leaks!
286-
when not defined(boehmGC) and not defined(nogc) and
287-
not defined(gcMarkAndSweep) and not defined(gogc) and
288-
not defined(gcRegions):
289-
if ntfNoRefs notin extGetCellType(result).base.flags:
290-
for i in newLen..result.len-1:
291-
forAllChildrenAux(dataPointer(result, elemAlign, elemSize, i),
292-
extGetCellType(result).base, waZctDecRef)
293-
294-
# XXX: zeroing out the memory can still result in crashes if a wiped-out
295-
# cell is aliased by another pointer (ie proc parameter or a let variable).
296-
# This is a tough problem, because even if we don't zeroMem here, in the
297-
# presence of user defined destructors, the user will expect the cell to be
298-
# "destroyed" thus creating the same problem. We can destroy the cell in the
299-
# finalizer of the sequence, but this makes destruction non-deterministic.
300-
zeroMem(dataPointer(result, elemAlign, elemSize, newLen), (result.len-%newLen) *% elemSize)
301-
result.len = newLen
302-
303-
proc setLengthSeqUninit(s: PGenericSeq, typ: PNimType, newLen: int, isTrivial: bool): PGenericSeq {.
304-
compilerRtl.} =
305-
sysAssert typ.kind == tySequence, "setLengthSeqUninit: type is not a seq"
269+
proc extendCapacityRaw(src: PGenericSeq; typ: PNimType;
270+
elemSize, elemAlign, newLen: int): PGenericSeq {.inline.} =
271+
## Reallocs `src` to fit `newLen` elements without any checks.
272+
## Capacity always increases to at least next `resize` step.
273+
let newCap = max(resize(src.space), newLen)
274+
result = cast[PGenericSeq](newSeq(typ, newCap))
275+
copyMem(dataPointer(result, elemAlign), dataPointer(src, elemAlign), src.len * elemSize)
276+
# since we steal the content from 's', it's crucial to set s's len to 0.
277+
src.len = 0
278+
279+
proc truncateRaw(src: PGenericSeq; baseFlags: set[TNimTypeFlag]; isTrivial: bool;
280+
elemSize, elemAlign, newLen: int): PGenericSeq {.inline.} =
281+
## Truncates `src` to `newLen` without any checks.
282+
## Does not set `src.len`
283+
# sysAssert src.space > newlen
284+
# sysAssert newLen < src.len
285+
result = src
286+
# we need to decref here, otherwise the GC leaks!
287+
when not defined(boehmGC) and not defined(nogc) and
288+
not defined(gcMarkAndSweep) and not defined(gogc) and
289+
not defined(gcRegions):
290+
if ntfNoRefs notin baseFlags:
291+
for i in newLen..<result.len:
292+
forAllChildrenAux(dataPointer(result, elemAlign, elemSize, i),
293+
extGetCellType(result).base, waZctDecRef)
294+
# XXX: zeroing out the memory can still result in crashes if a wiped-out
295+
# cell is aliased by another pointer (ie proc parameter or a let variable).
296+
# This is a tough problem, because even if we don't zeroMem here, in the
297+
# presence of user defined destructors, the user will expect the cell to be
298+
# "destroyed" thus creating the same problem. We can destroy the cell in the
299+
# finalizer of the sequence, but this makes destruction non-deterministic.
300+
if not isTrivial: # optimization for trivial types
301+
zeroMem(dataPointer(result, elemAlign, elemSize, newLen),
302+
((result.len-%newLen) *% elemSize))
303+
304+
template setLengthSeqImpl(s: PGenericSeq, typ: PNimType, newLen: int; isTrivial: bool;
305+
doInit: static bool) =
306306
if s == nil:
307-
if newLen == 0:
308-
result = s
309-
else:
310-
result = cast[PGenericSeq](newSeq(typ, newLen))
307+
if newLen == 0: return s
308+
else: return cast[PGenericSeq](newSeq(typ, newLen)) # newSeq zeroes!
311309
else:
312310
let elemSize = typ.base.size
313311
let elemAlign = typ.base.align
314-
if s.space < newLen:
315-
let r = max(resize(s.space), newLen)
316-
result = cast[PGenericSeq](newSeq(typ, r))
317-
copyMem(dataPointer(result, elemAlign), dataPointer(s, elemAlign), s.len * elemSize)
318-
# since we steal the content from 's', it's crucial to set s's len to 0.
319-
s.len = 0
320-
elif newLen < s.len:
321-
result = s
322-
# we need to decref here, otherwise the GC leaks!
323-
when not defined(boehmGC) and not defined(nogc) and
324-
not defined(gcMarkAndSweep) and not defined(gogc) and
325-
not defined(gcRegions):
326-
if ntfNoRefs notin typ.base.flags:
327-
for i in newLen..result.len-1:
328-
forAllChildrenAux(dataPointer(result, elemAlign, elemSize, i),
329-
extGetCellType(result).base, waZctDecRef)
330-
331-
# XXX: zeroing out the memory can still result in crashes if a wiped-out
332-
# cell is aliased by another pointer (ie proc parameter or a let variable).
333-
# This is a tough problem, because even if we don't zeroMem here, in the
334-
# presence of user defined destructors, the user will expect the cell to be
335-
# "destroyed" thus creating the same problem. We can destroy the cell in the
336-
# finalizer of the sequence, but this makes destruction non-deterministic.
337-
if not isTrivial: # optimization for trivial types
338-
zeroMem(dataPointer(result, elemAlign, elemSize, newLen), (result.len-%newLen) *% elemSize)
339-
else:
340-
result = s
312+
result = if newLen > s.space:
313+
s.extendCapacityRaw(typ, elemSize, elemAlign, newLen)
314+
elif newLen < s.len:
315+
s.truncateRaw(typ.base.flags, isTrivial, elemSize, elemAlign, newLen)
316+
else:
317+
when doInit:
318+
zeroMem(dataPointer(s, elemAlign, elemSize, s.len), (newLen-%s.len) *% elemSize)
319+
s
341320
result.len = newLen
342321

322+
proc setLengthSeqUninit(s: PGenericSeq; typ: PNimType; newLen: int; isTrivial: bool): PGenericSeq {.
323+
compilerRtl.} =
324+
sysAssert typ.kind == tySequence, "setLengthSeqUninit: type is not a seq"
325+
setLengthSeqImpl(s, typ, newLen, isTrivial, doInit = false)
326+
343327
proc setLengthSeqV2(s: PGenericSeq, typ: PNimType, newLen: int, isTrivial: bool): PGenericSeq {.
344328
compilerRtl.} =
345329
sysAssert typ.kind == tySequence, "setLengthSeqV2: type is not a seq"
346-
if s == nil:
347-
if newLen == 0:
348-
result = s
349-
else:
350-
result = cast[PGenericSeq](newSeq(typ, newLen))
351-
else:
352-
let elemSize = typ.base.size
353-
let elemAlign = typ.base.align
354-
if s.space < newLen:
355-
let r = max(resize(s.space), newLen)
356-
result = cast[PGenericSeq](newSeq(typ, r))
357-
copyMem(dataPointer(result, elemAlign), dataPointer(s, elemAlign), s.len * elemSize)
358-
# since we steal the content from 's', it's crucial to set s's len to 0.
359-
s.len = 0
360-
elif newLen < s.len:
361-
result = s
362-
# we need to decref here, otherwise the GC leaks!
363-
when not defined(boehmGC) and not defined(nogc) and
364-
not defined(gcMarkAndSweep) and not defined(gogc) and
365-
not defined(gcRegions):
366-
if ntfNoRefs notin typ.base.flags:
367-
for i in newLen..result.len-1:
368-
forAllChildrenAux(dataPointer(result, elemAlign, elemSize, i),
369-
extGetCellType(result).base, waZctDecRef)
370-
371-
# XXX: zeroing out the memory can still result in crashes if a wiped-out
372-
# cell is aliased by another pointer (ie proc parameter or a let variable).
373-
# This is a tough problem, because even if we don't zeroMem here, in the
374-
# presence of user defined destructors, the user will expect the cell to be
375-
# "destroyed" thus creating the same problem. We can destroy the cell in the
376-
# finalizer of the sequence, but this makes destruction non-deterministic.
377-
if not isTrivial: # optimization for trivial types
378-
zeroMem(dataPointer(result, elemAlign, elemSize, newLen), (result.len-%newLen) *% elemSize)
379-
else:
380-
result = s
381-
zeroMem(dataPointer(result, elemAlign, elemSize, result.len), (newLen-%result.len) *% elemSize)
382-
result.len = newLen
330+
setLengthSeqImpl(s, typ, newLen, isTrivial, doInit = true)
383331

384332
func capacity*(self: string): int {.inline.} =
385333
## Returns the current capacity of the string.
@@ -402,3 +350,4 @@ func capacity*[T](self: seq[T]): int {.inline.} =
402350

403351
let sek = cast[PGenericSeq](self)
404352
result = if sek != nil: sek.space else: 0
353+

0 commit comments

Comments
 (0)