increase performance by migrating from bytes.Buffer to strings.Builder#512
increase performance by migrating from bytes.Buffer to strings.Builder#512mna merged 1 commit intoPuerkitoBio:masterfrom
Conversation
|
Hello Saddam! Thanks for your interest in goquery, the idea is interesting but I don't want to merge a change just for change's sake, do you have benchmark results (bytes.Buffer vs strings.Builder in the various methods where you made the change) that demonstrate a measurable improvement? Thanks, |
|
I add benchmark to this. Please check commit 2aacd3d Edit: wait let me adjust the benchmark to test the changed code |
|
Sorry, I meant benchmarking the goquery methods affected by the change, not benchmarking raw I'll try to get some results on my side, by running the relevant benchmarks of the current version vs your changes. |
|
Some results on my old Thinkpad T460: A bit slower but less memory usage in the first benchmark, but it does seem like a bit of an improvement in the second. Unfortunately I don't have an existing benchmark for Thanks, |
|
Done |
|
One more: Again, ever so slightly slower but less memory usage. |
|
Thanks for your contribution! |
|
I got different result, hmmm package goquery
import (
"bytes"
"golang.org/x/net/html"
"strings"
"testing"
)
func BenchmarkStringsBuilderHtml(b *testing.B) {
// Find selector and reset timer
s := DocB().Find("body")
b.ResetTimer()
// Since there is no .innerHtml, the HTML content must be re-created from
// the nodes using html.Render.
var builder strings.Builder
if len(s.Nodes) > 0 {
for c := s.Nodes[0].FirstChild; c != nil; c = c.NextSibling {
err := html.Render(&builder, c)
if err != nil {
b.Fatal("Unexpected error:", err)
}
}
_ = builder.String()
}
}
func BenchmarkBytesBufferHtml(b *testing.B) {
// Find selector and reset timer
s := DocB().Find("body")
b.ResetTimer()
// Since there is no .innerHtml, the HTML content must be re-created from
// the nodes using html.Render.
var buf bytes.Buffer
if len(s.Nodes) > 0 {
for c := s.Nodes[0].FirstChild; c != nil; c = c.NextSibling {
err := html.Render(&buf, c)
if err != nil {
b.Fatal("Unexpected error:", err)
}
}
_ = buf.String()
}
}
func BenchmarkStringsBuilderText(b *testing.B) {
// Find selector and reset timer
s := DocB().Find("body")
b.ResetTimer()
var builder strings.Builder
// Slightly optimized vs calling Each: no single selection object created
var f func(*html.Node)
f = func(n *html.Node) {
if n.Type == html.TextNode {
// Keep newlines and spaces, like jQuery
builder.WriteString(n.Data)
}
if n.FirstChild != nil {
for c := n.FirstChild; c != nil; c = c.NextSibling {
f(c)
}
}
}
for _, n := range s.Nodes {
f(n)
}
_ = builder.String()
}
func BenchmarkBytesBufferText(b *testing.B) {
// Find selector and reset timer
s := DocB().Find("body")
b.ResetTimer()
var buf bytes.Buffer
// Slightly optimized vs calling Each: no single selection object created
var f func(*html.Node)
f = func(n *html.Node) {
if n.Type == html.TextNode {
// Keep newlines and spaces, like jQuery
buf.WriteString(n.Data)
}
if n.FirstChild != nil {
for c := n.FirstChild; c != nil; c = c.NextSibling {
f(c)
}
}
}
for _, n := range s.Nodes {
f(n)
}
_ = buf.String()
}
func BenchmarkStringsBuilderOuterHtml(b *testing.B) {
// Find selector and reset timer
s := DocB().Find("body")
b.ResetTimer()
var builder strings.Builder
if err := Render(&builder, s); err != nil {
b.Fatal("Unexpected error:", err)
}
_ = builder.String()
}
func BenchmarkBytesBufferOuterHtml(b *testing.B) {
// Find selector and reset timer
s := DocB().Find("body")
b.ResetTimer()
var buf bytes.Buffer
if err := Render(&buf, s); err != nil {
b.Fatal("Unexpected error:", err)
}
_ = buf.String()
} |
|
With those fractional nanosecond times, the compiler is probably optimizing your benchmark away. I already have benchmarks for Html, Text and the example, so I ran them before and after your changes. |
|
On closer look you're missing the |
|
ahh i see, now its working |
|
Yeah not an undisputable improvement for sure, but it feels cleaner since those are all building strings. |
No description provided.