gopher-lua
gopher-lua copied to clipboard
Refactor LTable to improve performance
Improve conversion speed between lua value and go value by reduce memory allocation。
Changes proposed in this pull request:
append lTableDictValue{value LValue, keyIndex int}. change LTable would contains unique dict map[LValue]lTableDictValue. change LTable#keys as []*LValue to reduce memory allocation on RawSetString. append LTable#DictionaryLen() (use to initial go map cap). append some convert functions between LValue and go value.
Benchmark testing:
- Load the 28kb json decode as map[string]interface{}, convert to LTable and then convert back.
- original BenchmarkToLValue-4 2157 559233 ns/op 272908 B/op 3799 allocs/op BenchmarkToGValue-4 3924 270173 ns/op 126799 B/op 2025 allocs/op
- changed BenchmarkToLValue-4 2704 377055 ns/op 187883 B/op 3459 allocs/op BenchmarkToGValue-4 6284 169170 ns/op 82971 B/op 981 allocs/op
I would charge test code, if the changing would be accepted.
Coverage decreased (-0.8%) to 87.286% when pulling dbe16f64515a2b4960708fb1d5954874d7c1d609 on shawnclovie:refactor/improve_pref into 6ff375d91eabb95a1fb05873aef638c2583f3704 on yuin:master.
current table implementation is even not correct in some cases, what about reviewing #283
I think that maybe this change:
" change LTable#keys as []*LValue to reduce memory allocation on RawSetString."
May not be doing what's wanted.
LValue
is an interface type, and as such is modelled behind the scenes as:
type iface struct {
tab *itab
data unsafe.Pointer
}
So a slice of LValues []LValue
is modelled in memory as:
*itab
unsafe.Pointer
*itab
unsafe.Pointer
*itab
*unsafe.Pointer
....
I think changing the type to be []*LValue
will actually change the slice to be an array of pointers to new allocated iface
structures on the heap, which will then point to the actual value. i.e.
*iface
*iface
*iface
....
These iface
elements will be heap allocated. In other words, it will introduce an additional redirection and also potentially spread the values around the heap so that linear iteration of the slice may potentially incur additional cache misses.
I did an investigation of slices of interface{}
a couple of years back and found it to be like this, I did a write up here: https://tul.github.io/2018/07/23/go-interfaces-deep-dive.html
So I actually think this particular aspect of the change may be detrimental to performance.