jyapayne/Nim
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

remove a condition that table size must be passed as power of 2 (#14926)

Browse source 
* remove a condition that table size must be passed as power of 2
* remove power-of-2 condition from sets and sharedtables
* remove power-of-2 condition from deques
* use 'correctSize' for both branches
* prettify changelog.md and fix typos
* add a changelog entry
* fix double-call of 'right-size'
* fix the same thing in sets.nim
* introduce a new internal proc `slotsNeeded`

Deprecate the public proc `rightSize`, which is not needed anymore.
Now it is an identity function, allowing the old code to work
correctly and without extra allocations.
This commit is contained in:
Miran 2020-07-08 15:01:47 +02:00 • committed by GitHub
commit 3de5296337
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 83 additions and 120 deletions
Download patch file Download diff file Expand all files Collapse all files

11
lib/pure/collections/deques.nim
View file

@ -67,9 +67,9 @@ const
defaultInitialSize* = 4
template initImpl(result: typed, initialSize: int) =
assert isPowerOfTwo(initialSize)
result.mask = initialSize-1
newSeq(result.data, initialSize)
let correctSize = nextPowerOfTwo(initialSize)
result.mask = correctSize-1
newSeq(result.data, correctSize)
template checkIfInitialized(deq: typed) =
when compiles(defaultInitialSize):
@ -82,11 +82,6 @@ proc initDeque*[T](initialSize: int = 4): Deque[T] =
## Optionally, the initial capacity can be reserved via `initialSize`
## as a performance optimization.
## The length of a newly created deque will still be 0.
##
## ``initialSize`` must be a power of two (default: 4).
## If you need to accept runtime values for this you could use the
## `nextPowerOfTwo proc<math.html#nextPowerOfTwo,int>`_ from the
## `math module<math.html>`_.
result.initImpl(initialSize)
proc len*[T](deq: Deque[T]): int {.inline.} =

14
lib/pure/collections/hashcommon.nim
View file

@ -30,17 +30,23 @@ proc nextTry(h, maxHash: Hash): Hash {.inline.} =
result = (h + 1) and maxHash
proc mustRehash[T](t: T): bool {.inline.} =
# If this is changed, make sure to synchronize it with `slotsNeeded` below
assert(t.dataLen > t.counter)
result = (t.dataLen * 2 < t.counter * 3) or (t.dataLen - t.counter < 4)
proc rightSize*(count: Natural): int {.inline.} =
proc slotsNeeded(count: Natural): int {.inline.} =
# Make sure to synchronize with `mustRehash` above
result = nextPowerOfTwo(count * 3 div 2 + 4)
proc rightSize*(count: Natural): int {.inline, deprecated: "Deprecated since 1.4.0".} =
## **Deprecated since Nim v1.4.0**, it is not needed anymore
## because picking the correct size is done internally.
##
## Return the value of ``initialSize`` to support ``count`` items.
##
## If more items are expected to be added, simply add that
## expected extra amount to the parameter before calling this.
#
# Make sure to synchronize with `mustRehash`
result = nextPowerOfTwo(count * 3 div 2 + 4)
result = count
template rawGetKnownHCImpl() {.dirty.} =
if t.dataLen == 0:

4
lib/pure/collections/setimpl.nim
View file

@ -16,12 +16,12 @@ template dataLen(t): untyped = len(t.data)
include hashcommon
template initImpl(s: typed, size: int) =
assert isPowerOfTwo(size)
let correctSize = slotsNeeded(size)
when s is OrderedSet:
s.first = -1
s.last = -1
s.counter = 0
newSeq(s.data, size)
newSeq(s.data, correctSize)
template rawInsertImpl() {.dirty.} =
if data.len == 0:

30
lib/pure/collections/sets.nim
View file

@ -94,11 +94,6 @@ include setimpl
proc init*[A](s: var HashSet[A], initialSize = defaultInitialSize) =
## Initializes a hash set.
##
## The `initialSize` parameter needs to be a power of two (default: 64).
## If you need to accept runtime values for this, you can use
## `math.nextPowerOfTwo proc <math.html#nextPowerOfTwo,int>`_ or
## `rightSize proc <#rightSize,Natural>`_ from this module.
##
## Starting from Nim v0.20, sets are initialized by default and it is
## not necessary to call this function explicitly.
##
@ -222,7 +217,7 @@ proc toHashSet*[A](keys: openArray[A]): HashSet[A] =
assert len(b) == 5
## b == {'a', 'b', 'c', 'd', 'r'}
result = initHashSet[A](rightSize(keys.len))
result = initHashSet[A](keys.len)
for key in items(keys): result.incl(key)
iterator items*[A](s: HashSet[A]): A =
@ -628,11 +623,6 @@ template forAllOrderedPairs(yieldStmt: untyped) {.dirty.} =
proc init*[A](s: var OrderedSet[A], initialSize = defaultInitialSize) =
## Initializes an ordered hash set.
##
## The `initialSize` parameter needs to be a power of two (default: 64).
## If you need to accept runtime values for this, you can use
## `math.nextPowerOfTwo proc <math.html#nextPowerOfTwo,int>`_ or
## `rightSize proc <#rightSize,Natural>`_ from this module.
##
## Starting from Nim v0.20, sets are initialized by default and it is
## not necessary to call this function explicitly.
##
@ -685,7 +675,7 @@ proc toOrderedSet*[A](keys: openArray[A]): OrderedSet[A] =
assert len(b) == 5
## b == {'a', 'b', 'r', 'c', 'd'} # different than in HashSet
result = initOrderedSet[A](rightSize(keys.len))
result = initOrderedSet[A](keys.len)
for key in items(keys): result.incl(key)
proc contains*[A](s: OrderedSet[A], key: A): bool =
@ -980,7 +970,7 @@ when isMainModule and not defined(release):
block toSeqAndString:
var a = toHashSet([2, 7, 5])
var b = initHashSet[int](rightSize(a.len))
var b = initHashSet[int](a.len)
for x in [2, 7, 5]: b.incl(x)
assert($a == $b)
#echo a
@ -1098,20 +1088,6 @@ when isMainModule and not defined(release):
b.incl(2)
assert b.len == 1
block:
type FakeTable = object
dataLen: int
counter: int
countDeleted: int
var t: FakeTable
for i in 0 .. 32:
var s = rightSize(i)
t.dataLen = s
t.counter = i
doAssert s > i and not mustRehash(t),
"performance issue: rightSize() will not elide enlarge() at: " & $i
block missingOrExcl:
var s = toOrderedSet([2, 3, 6, 7])
assert s.missingOrExcl(4) == true

8
lib/pure/collections/sharedtables.nim
View file

@ -207,15 +207,11 @@ proc len*[A, B](t: var SharedTable[A, B]): int =
withLock t:
result = t.counter
proc init*[A, B](t: var SharedTable[A, B], initialSize = 64) =
proc init*[A, B](t: var SharedTable[A, B], initialSize = 32) =
## creates a new hash table that is empty.
##
## This proc must be called before any other usage of `t`.
##
## `initialSize` needs to be a power of two. If you need to accept runtime
## values for this you could use the ``nextPowerOfTwo`` proc from the
## `math <math.html>`_ module or the ``rightSize`` proc from this module.
assert isPowerOfTwo(initialSize)
let initialSize = slotsNeeded(initialSize)
t.counter = 0
t.dataLen = initialSize
t.data = cast[KeyValuePairSeq[A, B]](allocShared0(

6
lib/pure/collections/tableimpl.nim
View file

@ -121,12 +121,12 @@ template ctAnd(a, b): bool =
else: false
template initImpl(result: typed, size: int) =
let correctSize = slotsNeeded(size)
when ctAnd(declared(SharedTable), type(result) is SharedTable):
init(result, size)
init(result, correctSize)
else:
assert isPowerOfTwo(size)
result.counter = 0
newSeq(result.data, size)
newSeq(result.data, correctSize)
when compiles(result.first):
result.first = -1
result.last = -1

48
lib/pure/collections/tables.nim
View file

@ -239,7 +239,7 @@ type
## <#newTable,int>`_.
const
defaultInitialSize* = 64
defaultInitialSize* = 32
# ------------------------------ helpers ---------------------------------
@ -288,12 +288,6 @@ proc enlarge[A, B](t: var Table[A, B]) =
proc initTable*[A, B](initialSize = defaultInitialSize): Table[A, B] =
## Creates a new hash table that is empty.
##
## ``initialSize`` must be a power of two (default: 64).
## If you need to accept runtime values for this you could use the
## `nextPowerOfTwo proc<math.html#nextPowerOfTwo,int>`_ from the
## `math module<math.html>`_ or the `rightSize proc<#rightSize,Natural>`_
## from this module.
##
## Starting from Nim v0.20, tables are initialized by default and it is
## not necessary to call this function explicitly.
##
@ -335,7 +329,7 @@ proc toTable*[A, B](pairs: openArray[(A, B)]): Table[A, B] =
let b = toTable(a)
assert b == {'a': 5, 'b': 9}.toTable
result = initTable[A, B](rightSize(pairs.len))
result = initTable[A, B](pairs.len)
for key, val in items(pairs): result[key] = val
proc `[]`*[A, B](t: Table[A, B], key: A): B =
@ -780,12 +774,6 @@ iterator allValues*[A, B](t: Table[A, B]; key: A): B =
proc newTable*[A, B](initialSize = defaultInitialSize): <//>TableRef[A, B] =
## Creates a new ref hash table that is empty.
##
## ``initialSize`` must be a power of two (default: 64).
## If you need to accept runtime values for this you could use the
## `nextPowerOfTwo proc<math.html#nextPowerOfTwo,int>`_ from the
## `math module<math.html>`_ or the `rightSize proc<#rightSize,Natural>`_
## from this module.
##
## See also:
## * `newTable proc<#newTable,openArray[]>`_ for creating a `TableRef`
## from a collection of `(key, value)` pairs
@ -1260,12 +1248,6 @@ template forAllOrderedPairs(yieldStmt: untyped) {.dirty.} =
proc initOrderedTable*[A, B](initialSize = defaultInitialSize): OrderedTable[A, B] =
## Creates a new ordered hash table that is empty.
##
## ``initialSize`` must be a power of two (default: 64).
## If you need to accept runtime values for this you could use the
## `nextPowerOfTwo proc<math.html#nextPowerOfTwo,int>`_ from the
## `math module<math.html>`_ or the `rightSize proc<#rightSize,Natural>`_
## from this module.
##
## Starting from Nim v0.20, tables are initialized by default and it is
## not necessary to call this function explicitly.
##
@ -1309,7 +1291,7 @@ proc toOrderedTable*[A, B](pairs: openArray[(A, B)]): OrderedTable[A, B] =
let b = toOrderedTable(a)
assert b == {'a': 5, 'b': 9}.toOrderedTable
result = initOrderedTable[A, B](rightSize(pairs.len))
result = initOrderedTable[A, B](pairs.len)
for key, val in items(pairs): result[key] = val
proc `[]`*[A, B](t: OrderedTable[A, B], key: A): B =
@ -1771,12 +1753,6 @@ iterator mvalues*[A, B](t: var OrderedTable[A, B]): var B =
proc newOrderedTable*[A, B](initialSize = defaultInitialSize): <//>OrderedTableRef[A, B] =
## Creates a new ordered ref hash table that is empty.
##
## ``initialSize`` must be a power of two (default: 64).
## If you need to accept runtime values for this you could use the
## `nextPowerOfTwo proc<math.html#nextPowerOfTwo,int>`_ from the
## `math module<math.html>`_ or the `rightSize proc<#rightSize,Natural>`_
## from this module.
##
## See also:
## * `newOrderedTable proc<#newOrderedTable,openArray[]>`_ for creating
## an `OrderedTableRef` from a collection of `(key, value)` pairs
@ -1803,7 +1779,7 @@ proc newOrderedTable*[A, B](pairs: openArray[(A, B)]): <//>OrderedTableRef[A, B]
let b = newOrderedTable(a)
assert b == {'a': 5, 'b': 9}.newOrderedTable
result = newOrderedTable[A, B](rightSize(pairs.len))
result = newOrderedTable[A, B](pairs.len)
for key, val in items(pairs): result.add(key, val)
@ -2251,12 +2227,6 @@ proc inc*[A](t: var CountTable[A], key: A, val: Positive = 1)
proc initCountTable*[A](initialSize = defaultInitialSize): CountTable[A] =
## Creates a new count table that is empty.
##
## ``initialSize`` must be a power of two (default: 64).
## If you need to accept runtime values for this you could use the
## `nextPowerOfTwo proc<math.html#nextPowerOfTwo,int>`_ from the
## `math module<math.html>`_ or the `rightSize proc<#rightSize,Natural>`_
## from this module.
##
## Starting from Nim v0.20, tables are initialized by default and it is
## not necessary to call this function explicitly.
##
@ -2269,7 +2239,7 @@ proc initCountTable*[A](initialSize = defaultInitialSize): CountTable[A] =
proc toCountTable*[A](keys: openArray[A]): CountTable[A] =
## Creates a new count table with every member of a container ``keys``
## having a count of how many times it occurs in that container.
result = initCountTable[A](rightSize(keys.len))
result = initCountTable[A](keys.len)
for key in items(keys): result.inc(key)
proc `[]`*[A](t: CountTable[A], key: A): int =
@ -2617,12 +2587,6 @@ proc inc*[A](t: CountTableRef[A], key: A, val = 1)
proc newCountTable*[A](initialSize = defaultInitialSize): <//>CountTableRef[A] =
## Creates a new ref count table that is empty.
##
## ``initialSize`` must be a power of two (default: 64).
## If you need to accept runtime values for this you could use the
## `nextPowerOfTwo proc<math.html#nextPowerOfTwo,int>`_ from the
## `math module<math.html>`_ or the `rightSize proc<#rightSize,Natural>`_
## from this module.
##
## See also:
## * `newCountTable proc<#newCountTable,openArray[A]>`_ for creating
## a `CountTableRef` from a collection
@ -2634,7 +2598,7 @@ proc newCountTable*[A](initialSize = defaultInitialSize): <//>CountTableRef[A] =
proc newCountTable*[A](keys: openArray[A]): <//>CountTableRef[A] =
## Creates a new ref count table with every member of a container ``keys``
## having a count of how many times it occurs in that container.
result = newCountTable[A](rightSize(keys.len))
result = newCountTable[A](keys.len)
for key in items(keys): result.inc(key)
proc `[]`*[A](t: CountTableRef[A], key: A): int =