s3: performance implications of s3fs

[isidentical]

This issue is for tracking the progress related s3fs/aiobotocore issues for the performance degradations that we might experience.

• exists() on non-existent files
• HeadObject calls are slower than the regular boto

[isidentical]

import time itself is not something that effects greatly, but still there is ~0.3 seconds (on HDD, might be different for a SSD user) startup overhead;

import boto3

import aiobotocore, boto3

[isidentical]

One important thing after I noticed working with the dataset-private-registry is that the listing take some time to happen since fsspec doesn't have iterators (not for ls/find) and I feel like this is bad for UX. For small sets, it is not that noticeable but if you have one million files then it is obvious that the dvc is blocked and there is no reactivity of any sort.

Regarding that, there is not much we can do on the dvc side though if we consider that the find() is mostly used for multi-level directories (like our remote cache) then we can do a first level ls() and then call find() with the each sub level. This definitely increases the responsiveness, and at the same time if we parallelize these calls I think it might be faster than the original walk_files() since now it is making N listings concurrently. CC: @efiop @shcheklein

[isidentical]

Just to mention some numbers with a similar implementation (these are obtained through a very WIP implementation, so take these with a pinch of salt):

• walk_files on the dataset-private-registry/ => 700-720 files/second with s3fs master
• walk_files on the dataset-private-registry/ => 870 files/second with the original implementation
• walk_files on the dataset-private-registry/ => 4446 files/second when ls() on the first level directories + find() on the each sublevel + pararellized with threading

    def walk_files(self, path_info, **kwargs):
        path = self._with_bucket(path_info)
        top_level_dirs = self.fs.ls(path)

        with ThreadPoolExecutor(max_workers=self.jobs) as executor:
            for results in executor.map(self.fs.find, top_level_dirs):
                for result in self._strip_buckets(results):
                    yield path_info.replace(path=result)

Of course, this optimization might slow you down when your cache have like 100-200 files in total separate across different prefixes, so we also have to consider that :hmm:

[efiop]

@isidentical My guess is that iterators should be there, I suppose they were simply not implemented yet. We should be able to implement them for us and possibly send the patch back to upstream, let's create an issue and ask about it.

[efiop]

Of course, this optimization might slow you down when your cache have like 100-200 files in total separate across different prefixes, so we also have to consider that :hmm:

We have a special optimization for that, where we try to list the whole remote if we estimate that it is smaller than a certain size. Though you've received pretty interesting results with ls, but that is not really walk_files anymore, more of a listing by prefix that we use for estimation.

[isidentical]

but that is not really walk_files anymore, more of a listing by prefix that we use for estimation.

What do you mean by that?

[efiop]

@isidentical Ah, sorry, I'm wrong. Didn't notice the find in the executor. Still, it is rather tailored to handle our specific remote dir structure, so it is no longer a general walk_files (that is expected to return an iterator) and we could use such a workaround directly where needed. The results are very interesting though.

[isidentical]

I started writing a proposal as an issue to the fsspec repo with describing the potential APIs and how would they work

[isidentical]

we have narrowed down the issue of when the region is not specified, the underlying boto implementation doing extra work to locate the bucket region which resulted with the aiohttp connection being halted down. so we end up paying the creation of the connection for each request, which is making stuff terrible.

aio-libs/aiobotocore#866 (comment)

we might implement some sort of bucket-region caching, though it is just basically a theory (didn't had time to actually test it out).

[isidentical]

See: fsspec/s3fs#495