apparent_encoding should be cached since chardet can be slow

Open
  • Is something wrong?
#6,250 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
48/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
python
Domain
api, performance

Research direction

Start at the Response.apparent_encoding property and its use from the r.text property. Reproduce repeated text accesses with content that requires detection, then inspect the existing response tests for a suitable regression test. Done means repeated accesses calculate the apparent encoding only once per response.

Written by the indexing model from the issue text.

Description

We have some scraper code that sometimes gets back PDFs and other times gets back HTML. Today we learned that if you access r.text in a large-ish PDF (40MB), chardet is called, which uses a lot of CPU (and a ton of memory):

r = requests.get(some_url)
r.text

That's more or less fine (best not to try to get the text of a PDF this way), but if you access r.text more than once, chardet gets run over and over.

We have code like this that performs horribly:

r = requests.get(some_url)
if bad_text in r.text:
    continue
if other_bad_text in r.text:
    continue
# ...many more tests...

When you access r.text, it checks if the encoding can come from the HTTP headers. If not, it runs the apparent_encoding property, which looks like:

    @property
    def apparent_encoding(self):
        """The apparent encoding, provided by the charset_normalizer or chardet libraries."""
        return chardet.detect(self.content)["encoding"]

I think that property should probably be cached since it's slow, so that repeated calls to r.text don't hurt so badly.

Expected Result

I expected the calls to the text property to only calculate the encoding once per request.

Actual Result

Each call to the text property re-calculates the encoding, which is slow and uses a lot of memory (this is probably a bug in chardet, but it uses hundreds of MB on a 40MB PDF right now).

System Information

$ python -m requests.help

{
  "chardet": {
    "version": "5.0.0"
  },
  "charset_normalizer": {
    "version": "2.0.12"
  },
  "cryptography": {
    "version": "36.0.2"
  },
  "idna": {
    "version": "2.10"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.10.7"
  },
  "platform": {
    "release": "5.4.209-116.363.amzn2.x86_64",
    "system": "Linux"
  },
  "pyOpenSSL": {
    "openssl_version": "101010ef",
    "version": "20.0.1"
  },
  "requests": {
    "version": "2.28.1"
  },
  "system_ssl": {
    "version": "101010ef"
  },
  "urllib3": {
    "version": "1.26.5"
  },
  "using_charset_normalizer": false,
  "using_pyopenssl": true
}
Dominant language
Python
Stars
54.7k
Forks
10.8k
Avg merge
18m
Merged PRs (30d)
1

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from psf/requests

All issues in psf/requests

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.