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

Support returning app_iter from renderer. #1417

Merged
merged 1 commit into from
Jan 6, 2015

Conversation

lrowe
Copy link
Contributor

@lrowe lrowe commented Sep 25, 2014

Sets response.app_iter = result for renderer results which are not text, bytes or None.

My motivating case here is that rendering very large json responses can lead to 'memory leaks' where python is unable to release the space required for the large string back to the OS. This causes process size to increase as it allocates first 100MB for a large json response, then later for a slightly larger json response, it allocates say 105MB, then 115MB... (These allocations are visible in /proc//smaps.)

My hope is that by using using json serializer that returns an iterable these large string allocations will not be required:

class JSONResult(object):
    def __init__(self):
        self.app_iter = []
        self.write = self.app_iter.append

    @classmethod
    def serializer(cls, value, **kw):
        fp = cls()
        json.dump(value, fp, **kw)
        return fp.app_iter


json_renderer = pyramid.renderers.JSON(serializer=JSONResult.serializer)

@mmerickel
Copy link
Member

I think this is great. I'd prefer to explicitly check if the result is iterable prior to assigning .app_iter in order to maintain the previous behavior that else is set to .bytes, however it looks like determining if something is actually iterable is almost intractable. It might be enough to use pyramid.compat.is_nonstr_iter. This is me just bringing it up to think about before merging. Thoughts, @lrowe?

@lrowe
Copy link
Contributor Author

lrowe commented Nov 10, 2014

That could work, I think we'd only need to test hasattr(result, '__iter__') as the str test of pyramid.compat.is_nonstr_iter is already checked first.

As webob.response.Response._body__set actively prevents you from setting non-bytes to response.body, swapping the order will preserve the TypeError which seems a good thing.

I'll try to look into making this change in the next few days.

@lrowe lrowe force-pushed the renderer_app_iter branch 2 times, most recently from ed8ddb1 to 2169c3c Compare December 6, 2014 01:56
Sets `response.app_iter = result` for renderer results which are iterable and not text/bytes.
@lrowe lrowe force-pushed the renderer_app_iter branch from 2169c3c to f0a9dfb Compare December 6, 2014 02:02
mmerickel added a commit that referenced this pull request Jan 6, 2015
Support returning app_iter from renderer.
@mmerickel mmerickel merged commit 469f410 into Pylons:master Jan 6, 2015
@mmerickel
Copy link
Member

Yay, thank you!

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.

2 participants