[Python/C++] S3FileSystem slow to deserialize due to AWS rule engine JSON parsing

[fjetter]

Describe the bug, including details regarding any error messages, version, and platform.

Deserializing a pickled S3FileSystem instance is surprisingly slow

import boto3
from pyarrow.fs import S3FileSystem

# Going via boto is not strictly necessary but setting all the keys and tokens already avoids one HTTP request during init
session = boto3.session.Session()
credentials = session.get_credentials()

fs = S3FileSystem(
    secret_key=credentials.secret_key,
    access_key=credentials.access_key,
    region="us-east-2",
    session_token=credentials.token,
)
# Note: This can also be seen by using just S3FileSystem() but this then posts one HTTP request and I want to emphasize the slow json parser, see below

%timeit pickle.loads(pickle.dumps(fs))

takes 1.01 ms ± 153 µs per loop on my machine

Looking at a py-spy profile shows that most of the time is spent in some internal JSON parsing. Is there a way to avoid this?

Component(s)

Python

[jorisvandenbossche]

cc @pitrou

[pitrou]

Looking at a py-spy profile shows that most of the time is spent in some internal JSON parsing. Is there a way to avoid this?

I have no idea. S3 configuration mechanisms are still a mystery to me.

[pitrou]

Similar observations here:

>>> %timeit s = S3FileSystem()
1.29 ms ± 1.27 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
>>> %timeit s = S3FileSystem(anonymous=True)
1.29 ms ± 5.52 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)
>>> %timeit s = S3FileSystem(anonymous=True, region='eu-west-1')
1.29 ms ± 4.86 µs per loop (mean ± std. dev. of 7 runs, 1,000 loops each)

[pitrou]

@fjetter Is it the repeated cost that bothers you? As in, if only the first S3FileSystem instantiation in a Python process was slow, would you mind?

[fjetter]

If the first instantiation is slow but repeated ones would reuse, etc. that would be fine. I'm running into this issue because those things are de/serialized as part of dask tasks, i.e. once per interpreter is fine, once per instance is not great

[pitrou]

Note to self: this is the construction path we go through currently:

S3Client::S3Client(const std::shared_ptr<AWSCredentialsProvider>& credentialsProvider,
                   const Client::ClientConfiguration& clientConfiguration,
                   Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy signPayloads /*= Never*/,
                   bool useVirtualAddressing /*= true*/,
                   Aws::S3::US_EAST_1_REGIONAL_ENDPOINT_OPTION USEast1RegionalEndPointOption) :
  BASECLASS(clientConfiguration,
            Aws::MakeShared<Aws::Auth::S3ExpressSignerProvider>(ALLOCATION_TAG,
                                                                credentialsProvider,
                                                                Aws::MakeShared<DefaultS3ExpressIdentityProvider>(ALLOCATION_TAG, *this),
                                                                SERVICE_NAME,
                                                                Aws::Region::ComputeSignerRegion(clientConfiguration.region),
                                                                signPayloads,
                                                                /*doubleEncodeValue*/ false),
            Aws::MakeShared<S3ErrorMarshaller>(ALLOCATION_TAG)),
    m_clientConfiguration(clientConfiguration, signPayloads, useVirtualAddressing, USEast1RegionalEndPointOption),
    m_executor(clientConfiguration.executor),
    m_endpointProvider(Aws::MakeShared<S3EndpointProvider>(ALLOCATION_TAG))
{
  init(m_clientConfiguration);
}

void S3Client::init(const S3::S3ClientConfiguration& config)
{
  AWSClient::SetServiceClientName("S3");
  AWS_CHECK_PTR(SERVICE_NAME, m_endpointProvider);
  m_endpointProvider->InitBuiltInParameters(config);
}

It's the Aws::MakeShared<S3EndpointProvider>(ALLOCATION_TAG) that takes so much time in practice.
We might cache the endpoint providers based on the config options that are used by InitBuiltInParameters, that is:

• in S3BuiltInParameters::SetFromClientConfiguration: config.useUSEast1RegionalEndPointOption, config.useArnRegion, config.disableMultiRegionAccessPoints, config.useVirtualAddressing
• in BuiltInParameters::SetFromClientConfiguration: config.region, config.useFIPS, config.useDualStack, config.endpointOverride, config.scheme

We would need at least the AWS SDK >= 1.10 to have S3ClientConfiguration and its additional fields.

[pitrou]

What is our minimum required AWS SDK version? @kou would you know the answer?

Edit: trying to find out in #40299

[pitrou]

With the caching PoC in #40299 I get the following:

>>> %timeit s = S3FileSystem(anonymous=True)
34.2 µs ± 42.8 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
>>> %timeit s = S3FileSystem(anonymous=True, region='eu-west-1')
32.8 µs ± 37.4 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
>>> %timeit s = S3FileSystem()
52.5 µs ± 262 ns per loop (mean ± std. dev. of 7 runs, 10,000 loops each)

[kou]

What is our minimum required AWS SDK version?

I think that we don't define it. Old AWS SDK can be used but there are some known problems. (I think that we recommend at least the current bundled version (1.10.55) to avoid known problems.)

If we want to define it, we can do it like the following:

diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index 951028b699..cf9232f331 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -5031,7 +5031,7 @@ macro(build_awssdk)
 endmacro()
 
 if(ARROW_S3)
-  resolve_dependency(AWSSDK HAVE_ALT TRUE)
+  resolve_dependency(AWSSDK HAVE_ALT TRUE REQUIRED_VERSION "X.Y.Z")
 
   message(STATUS "Found AWS SDK headers: ${AWSSDK_INCLUDE_DIR}")
   message(STATUS "Found AWS SDK libraries: ${AWSSDK_LINK_LIBRARIES}")

[pitrou]

I've marked #40299 ready for review.

[pitrou]

Issue resolved by pull request 40299
#40299