Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reducing memory consumption #2831

Merged
merged 5 commits into from Nov 28, 2023

Conversation

gluhar2006
Copy link
Contributor

I have found that the more requests Sanic processes, the more memory consumption increases. This behavior, as far as I could understand, is relevant only with a sufficiently large number of requests.
After some research, I took an idea from here to make sure that no unnecessary objects remain in memory after processing requests.

Application I used
import gc
import tracemalloc

from sanic import Sanic
from sanic.response import text
import psutil
import os

tracemalloc.start()


app = Sanic("MyHelloWorldApp")
request_cnt = 0

def mem_top():
  gc.collect()
  objs = gc.get_objects()

  return "\n".join((
      '',
      _top(10, 100, "\n", "{num} {obj}", (
          (len(gc.get_referents(obj)), obj) for obj in objs
      )),
  ))


def _top(limit, width, sep, fmt, nums_and_objs):
  return sep.join(
      fmt.format(num=num, type=type(obj), obj=repr(obj)[:width])
      for num, obj in sorted(nums_and_objs, key=lambda num_obj: -num_obj[0])[:limit]
  )


@app.post("/")
async def hello_world(request):
  global request_cnt
  request_cnt += 1
  print(round(psutil.Process(os.getpid()).memory_info().rss / 2 ** 30, 6))
  if request_cnt == 10000:
      print(str(mem_top()))
  return text("qq")


if __name__ == "__main__":
  app.run(port=5021)
I used ab to make requests

(it doesn't matter what's in the request body)

  ab -p data -T "application/x-www-form-urlencoded" -c 50 -n 10000 "http://0.0.0.0:5021/"

So i discovered thousands of TimerHandle HttpProtocol.check_timeouts and abort from SanicProtocol (required much more than 10000 requests).

My changes solves the memory freeing question, but it also has another side effect: here is rps from ab results before and after my changed - 7273 (main branch) vs 8558 (this branch).
Yes, I know that this is not the most relevant way to measure performance using such requests, but I don't have time to properly test the framework. I can only say that the applications that I develop and use Sanic at least did not slow down after my changes.

@gluhar2006 gluhar2006 requested a review from a team as a code owner October 4, 2023 12:21
@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (d1fc867) 88.414% compared to head (6771fac) 88.362%.

Files Patch % Lines
sanic/server/protocols/http_protocol.py 66.666% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##              main     #2831       +/-   ##
=============================================
- Coverage   88.414%   88.362%   -0.052%     
=============================================
  Files           92        92               
  Lines         7164      7175       +11     
  Branches      1229      1232        +3     
=============================================
+ Hits          6334      6340        +6     
- Misses         575       580        +5     
  Partials       255       255               

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Tronic
Copy link
Member

Tronic commented Oct 6, 2023

Should also look at clearing cyclic links once done with request/connection, so that Sanic itself would not leak memory when used without GC. Some of them are already cleared but surely some loops remain too.

@gluhar2006
Copy link
Contributor Author

@Tronic I didn't find any other issues of such type or memory leaks

@Tronic
Copy link
Member

Tronic commented Oct 23, 2023

@Tronic I didn't find any other issues of such type or memory leaks

But you still had GC enabled? Sanic takes care to clear to avoid cyclic links but I am sure it ain't 100 % yet. This is only relevant once GC is disabled.

@gluhar2006
Copy link
Contributor Author

gluhar2006 commented Oct 23, 2023

@Tronic

But you still had GC enabled? Sanic takes care to clear to avoid cyclic links but I am sure it ain't 100 % yet. This is only relevant once GC is disabled.

Yes, GC is enabled. I don't think that the basic recommendation of sanic usage is to disable GC

@Tronic
Copy link
Member

Tronic commented Oct 23, 2023

@Tronic

But you still had GC enabled? Sanic takes care to clear to avoid cyclic links but I am sure it ain't 100 % yet. This is only relevant once GC is disabled.

Yes, GC is enabled. I don't think that the basic recommendation of sanic usage is to disable GC

Definitely not a basic recommendation. It takes a lot of care to avoid/break cycles in all code, user apps included. But for maximal performance you would want it disabled, can make quite a difference at times.

@gluhar2006
Copy link
Contributor Author

@Tronic

But you still had GC enabled? Sanic takes care to clear to avoid cyclic links but I am sure it ain't 100 % yet. This is only relevant once GC is disabled.

Yes, GC is enabled. I don't think that the basic recommendation of sanic usage is to disable GC

Definitely not a basic recommendation. It takes a lot of care to avoid/break cycles in all code, user apps included. But for maximal performance you would want it disabled, can make quite a difference at times.

Memory consumption with disabled gc is low and stable

@ahopkins ahopkins merged commit 1310684 into sanic-org:main Nov 28, 2023
26 checks passed
@PushUpek PushUpek mentioned this pull request Jan 25, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants