From fa3620be9e9ca30bbfcfee25ee4ef85c1ece47f6 Mon Sep 17 00:00:00 2001 From: def Date: Tue, 3 Mar 2015 22:23:35 +0100 Subject: [PATCH 1/5] Only copy strings to their size, not capacity Capacity may be much bigger, so we end up with strings that are much larger than they have to be and have to copy more as well. --- lib/system/sysstr.nim | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/system/sysstr.nim b/lib/system/sysstr.nim index cfbc24f0cf..c0b337dcdc 100644 --- a/lib/system/sysstr.nim +++ b/lib/system/sysstr.nim @@ -77,13 +77,13 @@ proc copyString(src: NimString): NimString {.compilerRtl.} = if (src.reserved and seqShallowFlag) != 0: result = src else: - result = rawNewString(src.space) + result = rawNewString(src.len) result.len = src.len c_memcpy(result.data, src.data, (src.len + 1) * sizeof(char)) proc copyStringRC1(src: NimString): NimString {.compilerRtl.} = if src != nil: - var s = src.space + var s = src.len if s < 8: s = 7 when declared(newObjRC1): result = cast[NimString](newObjRC1(addr(strDesc), sizeof(TGenericSeq) + From f5968c79466d49c088916a9b07ae7417e0c4cc68 Mon Sep 17 00:00:00 2001 From: def Date: Wed, 4 Mar 2015 01:48:09 +0100 Subject: [PATCH 2/5] Only zero strings when necessary. This removes the zeroing when the string is subsequently overwritten by a memcpy anyway. --- lib/system/gc2.nim | 2 +- lib/system/sysstr.nim | 37 ++++++++++++++++++++++++------------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/lib/system/gc2.nim b/lib/system/gc2.nim index ae2d2c85d5..b8a61d6277 100644 --- a/lib/system/gc2.nim +++ b/lib/system/gc2.nim @@ -593,7 +593,7 @@ proc addNewObjToZCT(res: PCell, gch: var TGcHeap) {.inline.} = return add(gch.zct, res) -proc rawNewObj(typ: PNimType, size: int, gch: var TGcHeap, rc1: bool): pointer = +proc rawNewObj(typ: PNimType, size: int, gch: var TGcHeap, rc1 = false): pointer = # generates a new object and sets its reference counter to 0 acquire(gch) sysAssert(allocInv(gch.region), "rawNewObj begin") diff --git a/lib/system/sysstr.nim b/lib/system/sysstr.nim index c0b337dcdc..a8d5c1d977 100644 --- a/lib/system/sysstr.nim +++ b/lib/system/sysstr.nim @@ -35,13 +35,25 @@ proc eqStrings(a, b: NimString): bool {.inline, compilerProc.} = when declared(allocAtomic): template allocStr(size: expr): expr = cast[NimString](allocAtomic(size)) + + template allocStrNoInit(size: expr): expr = + cast[NimString](boehmAllocAtomic(size)) else: template allocStr(size: expr): expr = cast[NimString](newObj(addr(strDesc), size)) + template allocStrNoInit(size: expr): expr = + cast[NimString](rawNewObj(addr(strDesc), size, gch)) + +proc rawNewStringNoInit(space: int): NimString {.compilerProc.} = + var s = space + if s < 7: s = 7 + result = allocStrNoInit(sizeof(TGenericSeq) + s + 1) + result.reserved = s + proc rawNewString(space: int): NimString {.compilerProc.} = var s = space - if s < 8: s = 7 + if s < 7: s = 7 result = allocStr(sizeof(TGenericSeq) + s + 1) result.reserved = s @@ -53,10 +65,9 @@ proc copyStrLast(s: NimString, start, last: int): NimString {.compilerProc.} = var start = max(start, 0) var len = min(last, s.len-1) - start + 1 if len > 0: - result = rawNewString(len) + result = rawNewStringNoInit(len) result.len = len - c_memcpy(result.data, addr(s.data[start]), len * sizeof(char)) - #result.data[len] = '\0' + c_memcpy(result.data, addr(s.data[start]), (len + 1) * sizeof(char)) else: result = rawNewString(len) @@ -64,10 +75,9 @@ proc copyStr(s: NimString, start: int): NimString {.compilerProc.} = result = copyStrLast(s, start, s.len-1) proc toNimStr(str: cstring, len: int): NimString {.compilerProc.} = - result = rawNewString(len) + result = rawNewStringNoInit(len) result.len = len c_memcpy(result.data, str, (len+1) * sizeof(char)) - #result.data[len] = '\0' # readline relies on this! proc cstrToNimstr(str: cstring): NimString {.compilerRtl.} = result = toNimStr(str, c_strlen(str)) @@ -77,22 +87,23 @@ proc copyString(src: NimString): NimString {.compilerRtl.} = if (src.reserved and seqShallowFlag) != 0: result = src else: - result = rawNewString(src.len) + result = rawNewStringNoInit(src.len) result.len = src.len c_memcpy(result.data, src.data, (src.len + 1) * sizeof(char)) proc copyStringRC1(src: NimString): NimString {.compilerRtl.} = if src != nil: - var s = src.len - if s < 8: s = 7 when declared(newObjRC1): + var s = src.len + if s < 7: s = 7 result = cast[NimString](newObjRC1(addr(strDesc), sizeof(TGenericSeq) + s+1)) + result.reserved = s else: - result = allocStr(sizeof(TGenericSeq) + s + 1) - result.reserved = s + result = rawNewStringNoInit(src.len) result.len = src.len - c_memcpy(result.data, src.data, src.len + 1) + c_memcpy(result.data, src.data, (src.len + 1) * sizeof(char)) + proc hashString(s: string): int {.compilerproc.} = # the compiler needs exactly the same hash function! @@ -161,7 +172,7 @@ proc resizeString(dest: NimString, addlen: int): NimString {.compilerRtl.} = # DO NOT UPDATE LEN YET: dest.len = newLen proc appendString(dest, src: NimString) {.compilerproc, inline.} = - c_memcpy(addr(dest.data[dest.len]), src.data, src.len + 1) + c_memcpy(addr(dest.data[dest.len]), src.data, (src.len + 1) * sizeof(char)) inc(dest.len, src.len) proc appendChar(dest: NimString, c: char) {.compilerproc, inline.} = From 0264b42216cb92b7ac0a1b16e0b100a3a7221974 Mon Sep 17 00:00:00 2001 From: def Date: Wed, 4 Mar 2015 03:21:17 +0100 Subject: [PATCH 3/5] Also deepCopy strings only up to their length, not capacity --- lib/system/deepcopy.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/system/deepcopy.nim b/lib/system/deepcopy.nim index 7a26a17c54..19accf304f 100644 --- a/lib/system/deepcopy.nim +++ b/lib/system/deepcopy.nim @@ -34,7 +34,7 @@ proc genericDeepCopyAux(dest, src: pointer, n: ptr TNimNode) {.benign.} = proc copyDeepString(src: NimString): NimString {.inline.} = if src != nil: - result = rawNewString(src.space) + result = rawNewStringNoInit(src.len) result.len = src.len c_memcpy(result.data, src.data, (src.len + 1) * sizeof(char)) From d875951124c5d77b93eed48fbc92a7331f41e850 Mon Sep 17 00:00:00 2001 From: def Date: Wed, 4 Mar 2015 03:22:06 +0100 Subject: [PATCH 4/5] sizeof(char) is always 1 --- lib/system/deepcopy.nim | 2 +- lib/system/gc.nim | 2 +- lib/system/sysstr.nim | 16 ++++++++-------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/system/deepcopy.nim b/lib/system/deepcopy.nim index 19accf304f..093c0f3a7f 100644 --- a/lib/system/deepcopy.nim +++ b/lib/system/deepcopy.nim @@ -36,7 +36,7 @@ proc copyDeepString(src: NimString): NimString {.inline.} = if src != nil: result = rawNewStringNoInit(src.len) result.len = src.len - c_memcpy(result.data, src.data, (src.len + 1) * sizeof(char)) + c_memcpy(result.data, src.data, src.len + 1) proc genericDeepCopyAux(dest, src: pointer, mt: PNimType) = var diff --git a/lib/system/gc.nim b/lib/system/gc.nim index 1f4279c8f5..9de0c6503f 100644 --- a/lib/system/gc.nim +++ b/lib/system/gc.nim @@ -117,7 +117,7 @@ proc usrToCell(usr: pointer): PCell {.inline.} = # convert pointer to userdata to object (=pointer to refcount) result = cast[PCell](cast[ByteAddress](usr)-%ByteAddress(sizeof(TCell))) -proc canbeCycleRoot(c: PCell): bool {.inline.} = +proc canBeCycleRoot(c: PCell): bool {.inline.} = result = ntfAcyclic notin c.typ.flags proc extGetCellType(c: pointer): PNimType {.compilerproc.} = diff --git a/lib/system/sysstr.nim b/lib/system/sysstr.nim index a8d5c1d977..854c76d53e 100644 --- a/lib/system/sysstr.nim +++ b/lib/system/sysstr.nim @@ -30,7 +30,7 @@ proc eqStrings(a, b: NimString): bool {.inline, compilerProc.} = if a == b: return true if a == nil or b == nil: return false return a.len == b.len and - c_memcmp(a.data, b.data, a.len * sizeof(char)) == 0'i32 + c_memcmp(a.data, b.data, a.len) == 0'i32 when declared(allocAtomic): template allocStr(size: expr): expr = @@ -67,7 +67,7 @@ proc copyStrLast(s: NimString, start, last: int): NimString {.compilerProc.} = if len > 0: result = rawNewStringNoInit(len) result.len = len - c_memcpy(result.data, addr(s.data[start]), (len + 1) * sizeof(char)) + c_memcpy(result.data, addr(s.data[start]), len + 1) else: result = rawNewString(len) @@ -77,7 +77,7 @@ proc copyStr(s: NimString, start: int): NimString {.compilerProc.} = proc toNimStr(str: cstring, len: int): NimString {.compilerProc.} = result = rawNewStringNoInit(len) result.len = len - c_memcpy(result.data, str, (len+1) * sizeof(char)) + c_memcpy(result.data, str, len + 1) proc cstrToNimstr(str: cstring): NimString {.compilerRtl.} = result = toNimStr(str, c_strlen(str)) @@ -89,7 +89,7 @@ proc copyString(src: NimString): NimString {.compilerRtl.} = else: result = rawNewStringNoInit(src.len) result.len = src.len - c_memcpy(result.data, src.data, (src.len + 1) * sizeof(char)) + c_memcpy(result.data, src.data, src.len + 1) proc copyStringRC1(src: NimString): NimString {.compilerRtl.} = if src != nil: @@ -102,7 +102,7 @@ proc copyStringRC1(src: NimString): NimString {.compilerRtl.} = else: result = rawNewStringNoInit(src.len) result.len = src.len - c_memcpy(result.data, src.data, (src.len + 1) * sizeof(char)) + c_memcpy(result.data, src.data, src.len + 1) proc hashString(s: string): int {.compilerproc.} = @@ -124,7 +124,7 @@ proc addChar(s: NimString, c: char): NimString = if result.len >= result.space: result.reserved = resize(result.space) result = cast[NimString](growObj(result, - sizeof(TGenericSeq) + (result.reserved+1) * sizeof(char))) + sizeof(TGenericSeq) + result.reserved + 1)) result.data[result.len] = c result.data[result.len+1] = '\0' inc(result.len) @@ -168,11 +168,11 @@ proc resizeString(dest: NimString, addlen: int): NimString {.compilerRtl.} = result = cast[NimString](growObj(dest, sizeof(TGenericSeq) + sp + 1)) result.reserved = sp #result = rawNewString(sp) - #copyMem(result, dest, dest.len * sizeof(char) + sizeof(TGenericSeq)) + #copyMem(result, dest, dest.len + sizeof(TGenericSeq)) # DO NOT UPDATE LEN YET: dest.len = newLen proc appendString(dest, src: NimString) {.compilerproc, inline.} = - c_memcpy(addr(dest.data[dest.len]), src.data, (src.len + 1) * sizeof(char)) + c_memcpy(addr(dest.data[dest.len]), src.data, src.len + 1) inc(dest.len, src.len) proc appendChar(dest: NimString, c: char) {.compilerproc, inline.} = From 20426e77e9600eb78dc9b665b2a3fd5e088f06ae Mon Sep 17 00:00:00 2001 From: def Date: Wed, 4 Mar 2015 03:37:50 +0100 Subject: [PATCH 5/5] Fix copyStrLast to set the trailing \0 char --- lib/system/sysstr.nim | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/system/sysstr.nim b/lib/system/sysstr.nim index 854c76d53e..11780a9aa4 100644 --- a/lib/system/sysstr.nim +++ b/lib/system/sysstr.nim @@ -67,7 +67,8 @@ proc copyStrLast(s: NimString, start, last: int): NimString {.compilerProc.} = if len > 0: result = rawNewStringNoInit(len) result.len = len - c_memcpy(result.data, addr(s.data[start]), len + 1) + c_memcpy(result.data, addr(s.data[start]), len) + result.data[len] = '\0' else: result = rawNewString(len)