Optimize reading from S3

[mpenkov]

Fixes #317. Looks like content_length wasn't being cached in boto3.

With the updated code:

import sys

import boto3
import smart_open


def old_code(bucket, key, connection_params):
    s3 = boto3.resource('s3', **connection_params)
    obj = s3.Object(bucket, key)
    streamed_body = obj.get(Range='bytes=0-')['Body']
    return streamed_body.read(100)


def new_code(bucket, key, connection_params):
    f = smart_open.open(
        f's3://{bucket}/{key}', mode='rb',
        transport_params={'session': boto3.Session(**connection_params)},
    )
    return f.read(100)


def main():
    function_name, bucket, key = sys.argv[1:4]
    function = globals()[function_name]

    connection_params = {}
    data = function(bucket, key, connection_params)
    print(len(data))


if __name__ == '__main__':
    main()

(smart_open) misha@cabron:~/git/smart_open$ time python bug.py old_code bucket key
100

real    0m1.050s
user    0m0.274s
sys     0m0.041s
(smart_open) misha@cabron:~/git/smart_open$ time python bug.py new_code bucket key
100

real    0m1.275s
user    0m0.354s
sys     0m0.029s

We're now around 1.3 times slower than using boto3. I suspect that this is a start-up cost only, so as we read larger files, it will amortize.

[piskvorky]

Thanks, so the issue was due to multiple remote HEAD calls for content_length, right?

IMO 30% overhead is acceptable (plus there seems to be some ahead-of-time buffering in smart_open, which may be related too?).

[piskvorky]

What was happening before this change? Testing for a missing bucket?

[mpenkov]

Nonexisting key in an existing bucket.

The test started failing once we moved the content_length calls around. It's probably tripping over some moto oddity

[piskvorky]

@mpenkov our github repo badge is now red, "Build failing":

Traceback (most recent call last):
  File "/home/travis/build/RaRe-Technologies/smart_open/smart_open/tests/test_s3.py", line 385, in test_read_nonexisting_key
    create_bucket_and_key()
NameError: global name 'create_bucket_and_key' is not defined

Some issue with the modified test?

[mpenkov]

It may be a merge artifact. I'll have a look.