moovweb/gokogiri

performance issue of XmlNode.SetContent()

zhujinlonglbx opened this issue · 1 comments

Hi,

currently, XmlNode.SetContent() can both receive string/[]byte param for code conveniece,

func (xmlNode *XmlNode) SetContent(content interface{}) (err error) {
    switch data := content.(type) {
    default:
        err = ERR_UNDEFINED_SET_CONTENT_PARAM
    case string:
        err = xmlNode.SetContent([]byte(data))
    case []byte:
        contentBytes := GetCString(data)
        contentPtr := unsafe.Pointer(&contentBytes[0])
        C.xmlSetContent(unsafe.Pointer(xmlNode), unsafe.Pointer(xmlNode.Ptr), contentPtr)
    }
    return
}

But content.(type) and once more function call may cost more resource, below is my bechmark codes:

package fakeOverload

import "testing"

func SetContent(content interface{}) {
    switch data := content.(type) {
    case string:
        SetContent([]byte(data))
    case []byte:
        _ = content
    }
}

func SetContentBytes(bytes []byte) {
    _ = bytes
}

func BenchmarkSetContent(t *testing.B) {
    str := "test"
    for i := 0; i < t.N; i++ {
        SetContent(str)
    }
}

func BenchmarkSetContentBytes(t *testing.B) {
    str := "test"
    for i := 0; i < t.N; i++ {
        SetContentBytes([]byte(str))
    }
}

below is result:

PASS
BenchmarkSetContent testing: warning: no tests to run
50000000           260 ns/op          58 B/op          2 allocs/op
BenchmarkSetContentBytes    200000000           37.9 ns/op         8 B/op          0 allocs/op
ok      _/home/xxxx/benchmark/testFakeOverloadFunc  24.776s

we can see that if use SetContent() cost 6 times time and 7 times memory than SetContentBytes(),

how about use two functions:

SetContent(content string)
SetContentBytes(content []byte)

Thank you! :)

+1 for this. Gokogiri is a pretty low level api that may be called many times over in a loop and this should help with resource usage.