MixinNetwork / kraken

🐙 High performance WebRTC SFU implemented with pure Go.
Apache License 2.0
335 stars 49 forks source link

Why not delete room or peer if they are useless #32

Open maggch97 opened 2 years ago

maggch97 commented 2 years ago
type pmap struct {
    sync.RWMutex
    id string
    m  map[string]*Peer
}
type rmap struct {
    sync.RWMutex
    m map[string]*pmap
}

I found the peer and room data in these 2 struct will never be deleted. I think this memory leak is a problem if the service running for a long time. What do you think?

jadeydi commented 2 years ago

There is no limit the number of elements in map in golang. It does occupy more memory if service running for a long time, but the size of a blank map is small, I think there doesn't have much effect.

maggch97 commented 2 years ago

There is no limit the number of elements in map in golang. It does occupy more memory if service running for a long time, but the size of a blank map is small, I think there doesn't have much effect.

Sorry, I don't think the effect is small. Below is the info of kraken deployed in my server. There're only 6 active users but 2486 inactive users. The kraken service uses almost 500MB memory. When the service is just started, the memory usage was below 100MB.

The map is not blank also, all Peer struct are not released. But I'm not sure if the memory leak is caused by this.

{
   "data":{
      "updated_at":"2022-11-26T04:02:16.544191438Z",
      "active_peers":6,
      "closed_peers":2486,
      "active_rooms":4,
      "closed_rooms":119
   },
   "id":"1d870674-9329-44f7-b218-e40db615f52c"
}

image

jadeydi commented 2 years ago

There is no limit the number of elements in map in golang. It does occupy more memory if service running for a long time, but the size of a blank map is small, I think there doesn't have much effect.

Sorry, I don't think the effect is small. Below is the info of kraken deployed in my server. There're only 6 active users but 2486 inactive users. The kraken service uses almost 500MB memory. When the service is just started, the memory usage was below 100MB.

The map is not blank also, all Peer struct are not released. But I'm not sure if the memory leak is caused by this.

{
   "data":{
      "updated_at":"2022-11-26T04:02:16.544191438Z",
      "active_peers":6,
      "closed_peers":2486,
      "active_rooms":4,
      "closed_rooms":119
   },
   "id":"1d870674-9329-44f7-b218-e40db615f52c"
}

image

Thanks, will check and fix it

maggch97 commented 2 years ago

I used the change in this PR https://github.com/MixinNetwork/kraken/pull/33. The service uses only 40MB memory after running for more than 12 hours. Above screenshot is also captured after the service running about 12 hours.

This change solves the biggest memory leak problems.