bytedance / sonic

A blazingly fast JSON serializing & deserializing library
Apache License 2.0
6.54k stars 322 forks source link

使用sync.Pool管理buf的时候,高QPS可能会导致类似内存占用过大 #614

Closed szyhf closed 21 hours ago

szyhf commented 3 months ago

关联函数:

https://github.com/bytedance/sonic/blob/0704d0abfbfe81fe9d47b92963b87898a5f62aeb/internal/encoder/pools.go#L118-L121

https://github.com/bytedance/sonic/blob/0704d0abfbfe81fe9d47b92963b87898a5f62aeb/internal/encoder/pools.go#L128-L131


情况说明:

场景描述:

关于这个问题还可以参考官方包fmt类似场景的描述:

https://github.com/golang/go/blob/d8e47e257e40ab03c5eaf2316eaea4cb83e650c3/src/fmt/print.go#L160-L182

// free saves used pp structs in ppFree; avoids an allocation per invocation.
func (p *pp) free() {
    // Proper usage of a sync.Pool requires each entry to have approximately
    // the same memory cost. To obtain this property when the stored type
    // contains a variably-sized buffer, we add a hard limit on the maximum
    // buffer to place back in the pool. If the buffer is larger than the
    // limit, we drop the buffer and recycle just the printer.
    //
    // See https://golang.org/issue/23199.
    if cap(p.buf) > 64*1024 {
        p.buf = nil
    } else {
        p.buf = p.buf[:0]
    }
    if cap(p.wrappedErrs) > 8 {
        p.wrappedErrs = nil
    }

    p.arg = nil
    p.value = reflect.Value{}
    p.wrappedErrs = p.wrappedErrs[:0]
    ppFree.Put(p)
}

是同一类问题。


建议可以考虑

  1. 增加一个配置项,可以在特定需求下,拒绝复用cap大于指定长度的buf。
  2. fmt包一样直接写死一个大小,过大的直接弃用。
AsterDY commented 3 months ago

有选项的https://github.com/bytedance/sonic/blob/main/option/option.go#L21。一般生产实践这种导致的OOM只有在很高QPS的服务中才会见到。实际生产实践中更多的OOM问题是由于cache+NoCopyString导致的缓存浪费内存问题,也可以通过https://github.com/bytedance/sonic?tab=readme-ov-file#copy-string

szyhf commented 3 months ago

@AsterDY 感谢您的回复,不过我觉得不是同一个问题。

关于:

https://github.com/bytedance/sonic/blob/main/option/option.go#L21

这个配置项影响的是每个buff初始化的时候默认分配的buff大小。

但是在业务执行的序列化过程中,如果写入大的结构,会导致buf被扩容。

用完会在把buff游标归零后放回sync.Pool,这个场景下,其实分配的大buff还是存在的,只是由pool来控制释放时机。

特定场景下,pool里会持有大量被扩容到很大的buff,虽然节省了时间,但空间开支会很大。

所以我之前的提议是如果cap(buff)大于某个值,就不要放回sync.Pool了,直接丢弃让GC酌情回收(fmt也是这么干的)。


copy-string那边我看了下描述的是decoding string类型的字段的时候,生成的object里的string类型会持续引用json buff导致长期被持有不释放,跟我这个issue里提到的encoding的应该不是同一个问题。

szyhf commented 3 months ago

关联函数的改动思路:

https://github.com/bytedance/sonic/blob/0704d0abfbfe81fe9d47b92963b87898a5f62aeb/internal/encoder/pools.go#L118-L121

func freeBytes(p []byte) { 
    if cap(p) > OptionXXX {
        p = nil
        return
    }
     p = p[:0] 
     bytesPool.Put(p) 
 } 

https://github.com/bytedance/sonic/blob/0704d0abfbfe81fe9d47b92963b87898a5f62aeb/internal/encoder/pools.go#L128-L131

func freeBuffer(p *bytes.Buffer) { 
     if p.Cap() > OptionXXX {
         return
    }
     p.Reset() 
     bufferPool.Put(p) 
 } 
liuq19 commented 2 weeks ago

这个问题我们重新考虑一下,高QPS 可能出现内存浪费问题。

为了避免歧义,重新修改了标题。你说的这个问题准确来说应该不是内存泄露,因为 sync pool 本身保证内存了后面会被回收掉

szyhf commented 2 weeks ago

这个问题我们重新考虑一下,高QPS 可能出现内存浪费问题。

为了避免歧义,重新修改了标题。你说的这个问题准确来说应该不是内存泄露,因为 sync pool 本身保证内存了后面会被回收掉

谢谢回复。

是的,所以我原来的描述也是类似内存泄漏,本质上这些内存还是在管理中的。

另外就是这个问题其实不只是高QPS,主要是还涉及到,反复序列化反序列化特别大的json对象,大对象会产生大buf,然后高QPS环境下,大buf会慢慢淘汰小buf,最后每个小buf都被扩容成了大buf。

AsterDY commented 2 weeks ago

sync.Pool也会有GC,一般情况下不太可能导致OOM。目前我们公司内部没有出现过因为encoder buffer过大导致OOM的情况。你们如果有遇到的话可以分享一下,我们再根据实际情况来评估要不要优化

szyhf commented 2 weeks ago

我们这边确实就是线上遇到了,然后查了好久才找到问题在这,理论上sync.Pool确实应该有GC,但实际上当时就是线上这样跑,内存就会缓慢的涨,通过pprof也能看到内存大头就在这,最后就OOM了,最后在使用缓存池的地方加了类似fmt的处理以后,才好了。

我们的项目里,涉及大量的JSON序列化和反序列化的业务,主要是两块,一个是通讯消息,这里大部分都是小json,但调用非常频繁,另一个是数据存储,就有一些比较大的JSON(大的有几mb),我们实际测试,并在线上调整观察的情况,就是这些大的json会慢慢淘汰掉sync.Pool里的小buff,后期每个拿出来的buf都非常大,pprof的分析结果也会显示这块占用了大量内存。

参考fmt包里官方对sync.Pool使用的说明,我觉得这个场景应该是官方也知道的,我们最后就是参考这个写法处理的(线上就没再出问题了)。

https://github.com/golang/go/blob/d8e47e257e40ab03c5eaf2316eaea4cb83e650c3/src/fmt/print.go#L160-L182

主要是json序列化属于很基础的组件,就像fmt包里的设计一样,99%的情况这种设计确实可有可无,但一旦遇到这个场景,作为一个在项目大量地方调用底层库就非常难受了。

但确实也不是一个必选项,可能我们项目这种比较极端的情况才会出现,所以我也是希望可以作为一个可选项来处理,如果觉得每次都要额外判断一下大小会影响速度,可以考虑把当前的方法作为默认实现,然后允许通过配置项之类的,替换一个实现方法,应该对性能不会有很大的影响。

我们目前是fork了以后,用mod的replace替换了一下,项目的主库,依赖的很多自建的工具库,都涉及这个问题,所以才提这个建议,也希望你们能考虑下。

szyhf commented 2 weeks ago

还可以参考下这个issue

https://github.com/golang/go/issues/23199

chenzhuoyu commented 2 weeks ago

@szyhf 嗯,确实有一定道理。这个建议我们内部先讨论一下,看看怎么处理比较合适....