h2non / imaginary

Fast, simple, scalable, Docker-ready HTTP microservice for high-level image processing
https://fly.io/docs/app-guides/run-a-global-image-service/
MIT License
5.57k stars 456 forks source link

Question: Why debug.FreeOSMemory() #192

Closed reneklacan closed 6 years ago

reneklacan commented 6 years ago

Can I please ask you what's the main reason for having -mrelease option with the default value of 30.

Commit adding this feature is not really explaining anything https://github.com/h2non/imaginary/commit/cad8a2d4d149392ed0272fc81078625bf3d952c5

According to some sources FreeOSMemory might not be the best thing to run on production.

But I'm not a Go programmer so I'm wondering if you have production experience that suggests that periodically calling FreeOSMemory is a positive thing.

Dynom commented 6 years ago

I think this has something todo with the libvips bindings. Based on the commit I make the assumption that @h2non has run several benchmarks (The vegeta changes in the same commit) and that it seemed a good idea at that time.

However, to get a real answer you should wait for @h2non's reply :-)

h2non commented 6 years ago

@Dynom is right. Note that that commit has almost 3 years old, so things may changed since then.

At that time, I was running several exploratory benchmarks and found that some scenarios after stressing the server, the memory increased considerably without freeing up in a relatively short time. After some research and trial-error, I found that in cgo code, explicitly freeing memory had some benefits in order to decrease resident memory pressure in shorter periods.

Go runtime/GC has improved since then, so it might not have an obvious benefit doing that these days. That being said, the code has been running for quite a while and I don't know user reported issues that could be directly caused by this.

I guess doing some benchmarking/profiling can only clarify this.

reneklacan commented 6 years ago

Thank you for your answers.

Based on my experiments with 40 instances serving together ~400 requests per second tweaking -mrelease doesn't have any visible effect on memory consumption which is averaging at ~4.5GB per instance.