Fix descriptor.Table buffer growth calc #2311
Conversation
Fixes InsertAt size calculation to avoid growing slices more than needed. On insert the mask should calculate the index in the slice not the number of elements in the table. Avoids a factor of 8 increase in table size. Small optimisation to use the capcity of the slice to reduce the average length whilst keeping allocations small. Clarified the grow method. Signed-off-by: Edward McFarlane <emcfarlane@buf.build>
| @@ -109,10 +101,10 @@ func (t *Table[Key, Item]) InsertAt(item Item, key Key) bool { | |||
| if key < 0 { | |||
| return false | |||
| } | |||
| if diff := int(key) - t.Len(); diff > 0 { | |||
There was a problem hiding this comment.
For example, with 3 items in the table calling InsertAt(item, 64) would call grow(61) growing the internal slices of masks and items to 61 and 3904. Should be 2 and 128.
Signed-off-by: Edward McFarlane <emcfarlane@buf.build>
Signed-off-by: Edward McFarlane <emcfarlane@buf.build>
emcfarlane
force-pushed
the
ed/descriptorGrow
branch
from
e3bfec4
to
4d84665
Compare
|
@mathetake added a testcase which shows the error and panic on main on edgecases. Renamed the file but its causing a large diff then intended, let me know if you;d like a diff format, |
|
sorry what was the intent for renaming the file and adding another test? I would prefer make it in the existing _test.go |
|
Its a The new testcase ideally would like to assert the internal structure to exercise that grow is bounded correctly. |
Signed-off-by: Edward McFarlane <emcfarlane@buf.build>
|
hah i see |
|
ok i will defer to @achille-roussel for the rest. Thanks for the work here! |
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
|
pushed two commits to minimize the change - used the export_test pattern |
Fixes InsertAt size calculation to avoid growing slices more than needed. On insert the mask should calculate the index in the slice not the number of elements in the table. Avoids a factor of 8 increase in table size. Small optimisation to use the capcity of the slice to reduce the average length whilst keeping allocations down. Clarified the grow method grows the table by n * 64 items.