Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Reducing memory footprint ofRequest object: __slots__ and lazy evaluation#7036

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
albertedwardson wants to merge2 commits intoscrapy:master
base:master
Choose a base branch
Loading
fromalbertedwardson:request-obj

Conversation

@albertedwardson
Copy link

@albertedwardsonalbertedwardson commentedSep 1, 2025
edited
Loading

This PR makes such changes:

  1. Python’sdict objects are fairly heavy, even when empty. TheRequest object had a few of them. This PR makes their creation lazy (along with theflags list), so they are only created when they actually hold data. Previously, they were also created lazily, but once accessed, theRequest object would keep references to emptydicts/lists.In practice, these could just beNone instead. No, this breaks things. E.g.r.cookies[k]=v will return newly created empty dict, assignv tok key, and then discard this dict object since there is no references to it. Once accessed, they will be created and keeped. But when trying to assign empty dict or list to some ofRequest attributes, we won't store them.

  2. Added__slots__ toRequest class and its subclasses

Before

Python3.13.3 (main,May222025,01:58:53) [MSCv.194364bit (AMD64)]onwin32>>>frompympler.asizeofimportasizeof>>>fromscrapyimportRequest>>>asizeof(Request("http://example.com"))1480>>>asizeof(Request("http://example.com"))1472>>>asizeof(Request("http://example.com"))>>> ...# internal python magic/interning of some objects???>>>asizeof(Request("http://example.com"))1368# final size

After

Python3.13.3 (main,May222025,01:58:53) [MSCv.194364bit (AMD64)]onwin32Type"help","copyright","credits"or"license"formoreinformation.>>>frompympler.asizeofimportasizeof>>>fromscrapyimportRequest>>>r=Request("http://example.com")>>>asizeof(r)432>>>r.cookies# sets empty dict{}>>>asizeof(r)496>>>r.cookies= {}# sets None>>>asizeof(r)432

@albertedwardsonalbertedwardson changed the titleReducing memory foorprint ofRequest object: __slots__ and lazy evaluationReducing memory footprint ofRequest object: __slots__ and lazy evaluationSep 1, 2025
@wRAR
Copy link
Member

wRAR commentedSep 1, 2025

I think this broke tests.

@albertedwardson
Copy link
Author

albertedwardson commentedSep 1, 2025
edited
Loading

Oh, sorry, my bad. When I tested locally, I assumed the hanging test case was broken because I was running tests on my work PC with a weird Windows setup and a custom-compiled Python build

@albertedwardson
Copy link
Author

All unit tests of "request" objects now passing. But I didn't ran full suite locally, can't do it now.

@codecov
Copy link

codecovbot commentedSep 2, 2025
edited
Loading

Codecov Report

❌ Patch coverage is83.33333% with7 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.95%. Comparing base (a0b766f) to head (a831f2b).
⚠️ Report is 12 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing linesPatch %Lines
scrapy/http/request/__init__.py82.05%7 Missing⚠️
Additional details and impacted files
@@            Coverage Diff             @@##           master    #7036      +/-   ##==========================================+ Coverage   90.68%   90.95%   +0.27%==========================================  Files         164      164                Lines       12642    12788     +146       Branches     1643     1663      +20     ==========================================+ Hits        11464    11631     +167+ Misses        891      873      -18+ Partials      287      284       -3
Files with missing linesCoverage Δ
scrapy/http/request/form.py99.22% <100.00%> (+<0.01%)⬆️
scrapy/http/request/json_request.py100.00% <100.00%> (ø)
scrapy/http/request/rpc.py100.00% <100.00%> (ø)
scrapy/http/request/__init__.py92.48% <82.05%> (-4.46%)⬇️

... and25 files with indirect coverage changes

@albertedwardson

This comment was marked as resolved.

@albertedwardson
Copy link
Author

@wRAR is there any point for writing tests for setters?

@wRAR
Copy link
Member

wRAR commentedSep 3, 2025

is there any point for writing tests for setters?

Yeah, I think it's useful to make sure the attributes can be assigned, for these and future changes.

albertedwardson reacted with thumbs up emoji

albertedwardson added a commit to albertedwardson/scrapy that referenced this pull requestOct 5, 2025
this is mostly ai generatedidk those comments and docstring are helpful imo
albertedwardson added a commit to albertedwardson/scrapy that referenced this pull requestOct 5, 2025
this is mostly ai generatedidk those comments and docstring are helpful imo

@meta.setter
defmeta(self,value:dict[str,Any]|None)->None:
self._meta=valueifvalueelseNone

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I think this line can be likeself._meta = value or None.
Becouse{} or None -> None, so zero-value of dict can be ignored.

albertedwardson reacted with thumbs up emoji
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@AmirRedHatAmirRedHatAmirRedHat left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@albertedwardson@wRAR@AmirRedHat

[8]ページ先頭

©2009-2025 Movatter.jp