inconshreveable / log15

Structured, composable logging for Go
https://godoc.org/github.com/inconshreveable/log15
Other
1.1k stars 145 forks source link

Don't allocate a byte buffer if we don't need to escape #88

Closed kevinburke closed 8 years ago

kevinburke commented 8 years ago

escapeString is expensive. Let's do one pass to check whether anything needs to be escaped - which may be a common case - then a second pass to allocate a buffer and do the escaping, if we need to.

benchmark                        old ns/op     new ns/op     delta
BenchmarkStreamNoCtx-4           4798          4352          -9.30%
BenchmarkDiscard-4               823           805           -2.19%
BenchmarkCallerFileHandler-4     1880          1864          -0.85%
BenchmarkCallerFuncHandler-4     1728          1655          -4.22%
BenchmarkLogfmtNoCtx-4           3602          3165          -12.13%
BenchmarkJsonNoCtx-4             1647          1687          +2.43%
BenchmarkMultiLevelFilter-4      864           835           -3.36%
BenchmarkDescendant1-4           836           802           -4.07%
BenchmarkDescendant2-4           862           827           -4.06%
BenchmarkDescendant4-4           949           891           -6.11%
BenchmarkDescendant8-4           974           965           -0.92%
inconshreveable commented 8 years ago

cool! i wonder if there are some additional wins we could make:

  1. if no characters need to be escaped, but we need quotes (a very common case), we can copy the entire value in one call without a for loop which should allow the underlying implementation to use an optimized call like memcpy
  2. use a switch statement instead of if with boolean ORs
  3. if needQuotes and needEscape both become true, skip the remaining characters
tg commented 8 years ago

Also casting string to bytes and vice versa (return string(e.Bytes()[start:stop])) could be easily avoided by writing quotes only if needsQuotes is set and returning e.String() directly.

inconshreveable commented 8 years ago

@tg yeah, you're absolutely right. we should actually see if we can rewrite the formatter entirely to work entirely in bytes if possible to avoid any unnecessary conversions

ChrisHines commented 8 years ago

I don't want to put a damper on all the great work being done here, but another alternative is to use the github.com/go-logfmt/logfmt package which has many of these ideas already incorporated. PRs on that package welcome for additional speedups.

inconshreveable commented 8 years ago

argh! I had completely forgotten that you built that fantastic package. we should definitely be adopting that and not duplicate effort here =)