cache proxy configuration to get 12.5% speedup

I didn't plan to submit a PR, I was just playing around, but the speedup I got (granted, on a very superficial benchmark) was so high that I just had to try.

On my Macbook Air M1, 12.5% of runtime of a simplistic benchmark (see below) was spent on calls to `should_bypass_proxies()` which seems to call some OS-dependent code to get proxy bypass config, and I figure in most use cases it will be called repeatedly with a small number of hostnames and it's 100% ok to cache it (*is it?* I'm not an expert in `requests` use cases and maybe some people expect the OS config to change while the process is running and would consider this a breaking change).

Did my best to write clear code and document it well. Wrote tests. Code passes tox (except for py3.7 that I don't have installed).

Before:
```
         1145706 function calls (1139516 primitive calls) in 1.041 seconds

   Ordered by: internal time
   List reduced from 1546 to 10 due to restriction <10>

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
      800    0.261    0.000    0.261    0.000 {method 'recv_into' of '_socket.socket' objects}
      100    0.060    0.001    0.060    0.001 encoder.py:204(iterencode)
      200    0.056    0.000    0.057    0.000 {built-in method _socket.getaddrinfo}
      200    0.053    0.000    0.053    0.000 {built-in method _socket.gethostbyname}
      100    0.042    0.000    0.042    0.000 decoder.py:343(raw_decode)
      200    0.041    0.000    0.041    0.000 {built-in method _scproxy._get_proxy_settings}
     3800    0.034    0.000    0.041    0.000 shlex.py:133(read_token)
      200    0.030    0.000    0.030    0.000 {built-in method _scproxy._get_proxies}
      200    0.028    0.000    0.028    0.000 {method 'connect' of '_socket.socket' objects}
      200    0.024    0.000    0.025    0.000 {built-in method io.open}
```

After:

```
         916904 function calls (910715 primitive calls) in 0.920 seconds

   Ordered by: internal time
   List reduced from 1533 to 10 due to restriction <10>

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
      800    0.258    0.000    0.258    0.000 {method 'recv_into' of '_socket.socket' objects}
      200    0.064    0.000    0.064    0.000 {built-in method _socket.getaddrinfo}
      100    0.061    0.001    0.061    0.001 encoder.py:204(iterencode)
      100    0.042    0.000    0.042    0.000 decoder.py:343(raw_decode)
      200    0.035    0.000    0.035    0.000 {built-in method _scproxy._get_proxies}
     3800    0.034    0.000    0.041    0.000 shlex.py:133(read_token)
      200    0.026    0.000    0.026    0.000 {method 'connect' of '_socket.socket' objects}
      200    0.025    0.000    0.026    0.000 {built-in method io.open}
    26/22    0.021    0.001    0.023    0.001 {built-in method _imp.create_dynamic}
        1    0.015    0.015    0.799    0.799 bench.py:3(test)
```

Note these lines in particular:

```
      200    0.064    0.000    0.064    0.000 {built-in method _socket.getaddrinfo}
      200    0.041    0.000    0.041    0.000 {built-in method _scproxy._get_proxy_settings}
```

This is the benchmark I used:

`bench.py`:

```python3
import requests

def test():
    for _ in (range(10)):
        session = requests.Session()
        for _ in range(10):
            response = session.get('http://localhost:8000/home', params={'q': 'who are you?'})
            response.raise_for_status()
            response.content

            response = session.post('http://localhost:8000/api', json={'type': 'query', 'data': list(range(10000))})
            response.raise_for_status()
            response.json()

if __name__ == '__main__':
    test()
```

`bench_server.py`:

```python3
from flask import Flask, request

app = Flask(__name__)

@app.route('/home')
def home():
    return 'Hello, World! ' + 'A' * 10000

@app.route('/api', methods=['POST'])
def api():
    return request.json

if __name__ == '__main__':
    app.run(port=8000)
```

```
gunicorn bench_server:app
```

```
python -m timeit 'import bench; bench.test()'
```

```
(requests) ➜  requests git:(cache-proxy-config) ✗ ./toxin.sh -e py38-default "python -m timeit 'import bench; bench.test()'"
1 loop, best of 5: 654 msec per loop
(requests) ➜  requests git:(cache-proxy-config) ✗ ./toxin.sh -e py38-default "python -m timeit 'import bench; bench.test()'"
1 loop, best of 5: 664 msec per loop
(requests) ➜  requests git:(cache-proxy-config) ✗ ./toxin.sh -e py38-default "python -m timeit 'import bench; bench.test()'"
1 loop, best of 5: 685 msec per loop
(requests) ➜  requests git:(cache-proxy-config) ✗ ./toxin.sh -e py38-default "python -m timeit 'import bench; bench.test()'"
1 loop, best of 5: 750 msec per loop
(requests) ➜  requests git:(cache-proxy-config) ✗ ./toxin.sh -e py38-default "python -m timeit 'import bench; bench.test()'"
1 loop, best of 5: 759 msec per loop
(requests) ➜  requests git:(cache-proxy-config) ✗ ./toxin.sh -e py38-default "python -m timeit 'import bench; bench.test()'"
1 loop, best of 5: 770 msec per loop
(requests) ➜  requests git:(cache-proxy-config) ✗ python -c 'print(1 - 664/759)'
0.12516469038208167```

Hi @SonOfLilit,

Thanks for the suggestion! I'm not sure I'm seeing conclusive data for the improvements claim though. Requests has a fairly diverse user population. There are several variables we'd need to weigh including frequency of underlying configuration changes, number of proxies/configurations used and operating system before we can make any conclusions on performance.

There may be some minor performance improvements to be had, but introduction of a cache also introduces a new set of problems. Any changes in the underlying OS configurations may be ignored and there's no reasonable way to force cache eviction with this approach. We're also largely under a feature freeze, so I think it's unlikely we'll accept this at this time.

Thanks for the quick response.

Eviction is not reasonable to support here, but I don't see any reason to - would developers expect an existing session to react to changes in proxy bypass OS configuration? I'd expect to need to restart the process in this case, not just the session.

The performance claims are indeed Mac-only at this time. If I try this on Windows and Linux and see consistent >10% gains, will it make a difference?

Also, need help maintaining this project? It's my favorite python package since the day it came out, I can find some time in my day to do some code reviews.

> Eviction is not reasonable to support here, but I don't see any reason to [...] I'd expect to need to restart the process in this case, not just the session.

So that's an assumption, but it's not a data point without validation. The first concerns would be resolution from the[ Windows Registry](https://github.com/psf/requests/blob/main/requests/utils.py#L68-L116) and macOS system configurations via [SCDynamicCopyStoreProxies](https://developer.apple.com/documentation/systemconfiguration/1517088-scdynamicstorecopyproxies) ([cpython ref](https://github.com/python/cpython/blob/main/Modules/_scproxy.c#L171-L216)). I'm not claiming these are dynamic during a process' lifecycle because I haven't tested it. It would not surprise me if both are queried in real time and subject to change.

> The performance claims are indeed Mac-only at this time.

The data currently is for a fairly naive idea of how proxy configurations work. They demonstrate a single use-case experiences improvement at the potential cost of incorrect data. We'll always optimize for correctness over speed.

There's likely an eventuality where we could work through this to reach a conclusion but I don't think it's worth the time trade-off to do it right.

> Also, need help maintaining this project?

We appreciate the offer but I don't think code reviewing is the current bottleneck. All changes need to be weighed for potential impact and added complexity to the library. For the time being, we're relatively OK with the libraries current functionality and don't intend to make any significant changes.

On Thu, Feb 23, 2023, 01:20 Nate Prewitt ***@***.***> wrote:

> Eviction is not reasonable to support here, but I don't see any reason to
> [...] I'd expect to need to restart the process in this case, not just the
> session.
>
> So that's an assumption, but it's not a data point without validation.
> [..] I'm not claiming these are dynamic during a process' lifecycle because
> I haven't tested it. It would not surprise me if both are queried in real
> time and subject to change.
>
Oh no, I'm almost sure they're queried in real time and subject to change.
The assumption is about user expectations. I'm essentially saying "the
current behavior is probably perceived as a bit more buggy than the
proposed change". I know this is an always-wrong thing to say, but I'd be
surprised if any users ever considered what behavior they want in this
case, other than an obligatory xkcd.com/1172 guy of course.

We appreciate the offer but I don't think code reviewing is the current
> bottleneck. All changes need to be weighed for potential impact and added
> complexity to the library.
>
Yeah, that's what I meant by code review for a library that the entire
software world depends on.

Alright, I'll stop taking your time, thanks for doing all this!

> —
> Reply to this email directly, view it on GitHub
> <https://github.com/psf/requests/pull/6360#issuecomment-1440989131>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAADU7NMXZNNR6WSALOSF5LWY2NMXANCNFSM6AAAAAAVE4DRTQ>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>