Repository navigation
PERF/API: Specify datasource size to avoid HEAD requests for S3 endpoints #22739
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
4dfc295
ed52e1d
72c757e
27aadc0
a65f65d
3152205
b087627
2f34b6e
ddc0e31
7887199
62e8221
e0c4596
56df08a
e76a02b
a8afc20
e32eedd
2c4a077
5b504da
e9ed5b2
dcda6e1
60a5ecd
b460be2
e20c06d
8c6f01a
4f89dc5
e65fb89
82c2f19
c967399
0bd481e
b375237
c8a15b1
1bd862e
307b589
7dbe50a
9a13ecd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2019-2025, NVIDIA CORPORATION. | ||
| * SPDX-FileCopyrightText: Copyright (c) 2019-2026, NVIDIA CORPORATION. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
|
|
@@ -310,6 +310,17 @@ constexpr inline auto is_byte_like_type() | |
| std::is_same_v<non_cv_T, std::byte>; | ||
| } | ||
|
|
||
| /** | ||
| * @brief A file path with an optional known size in bytes. | ||
| * | ||
| * When `size` is set for a remote URL, libcudf passes it to KvikIO at open time so the remote | ||
| * server is not queried for file size. | ||
| */ | ||
|
TomAugspurger marked this conversation as resolved.
|
||
| struct filepath_source { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about the name
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm I was hoping for some kind of convention like a "source" is a thing that goes into a If we change it to |
||
| std::string path; ///< Path or URL of the input file | ||
| std::optional<std::size_t> size{}; ///< Known file size; omit to query via KvikIO | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Source information for read interfaces | ||
| */ | ||
|
|
@@ -325,8 +336,13 @@ struct source_info { | |
| * @param file_paths Input files paths | ||
| */ | ||
| explicit source_info(std::vector<std::string> file_paths) | ||
| : _type(io_type::FILEPATH), _num_sources(file_paths.size()), _filepaths(std::move(file_paths)) | ||
| : _type(io_type::FILEPATH), _num_sources(file_paths.size()) | ||
|
TomAugspurger marked this conversation as resolved.
|
||
| { | ||
| _filepath_sources.reserve(file_paths.size()); | ||
| for (auto& path : file_paths) { | ||
| _filepath_sources.push_back({std::move(path), std::nullopt}); | ||
| } | ||
| rebuild_filepaths(); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -335,10 +351,21 @@ struct source_info { | |
| * @param file_path Single input file | ||
| */ | ||
| explicit source_info(std::string file_path) | ||
| : _type(io_type::FILEPATH), _num_sources(1), _filepaths({std::move(file_path)}) | ||
| : source_info(std::vector<std::string>{std::move(file_path)}) | ||
| { | ||
| } | ||
|
|
||
| /** | ||
| * @brief Construct a new source info object from filepath sources with optional known sizes | ||
| * | ||
| * @param sources Input filepath sources | ||
| */ | ||
| explicit source_info(std::vector<filepath_source> sources) | ||
| : _type(io_type::FILEPATH), _num_sources(sources.size()), _filepath_sources(std::move(sources)) | ||
| { | ||
| rebuild_filepaths(); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Construct a new source info object for multiple buffers in host memory | ||
| * | ||
|
|
@@ -424,6 +451,12 @@ struct source_info { | |
| * @return The type of the input | ||
| */ | ||
| [[nodiscard]] auto type() const { return _type; } | ||
| /** | ||
| * @brief Get the filepath sources of the input | ||
| * | ||
| * @return The filepath sources of the input | ||
| */ | ||
| [[nodiscard]] auto const& filepath_sources() const { return _filepath_sources; } | ||
| /** | ||
| * @brief Get the filepaths of the input | ||
| * | ||
|
|
@@ -457,8 +490,18 @@ struct source_info { | |
| [[nodiscard]] auto num_sources() const { return _num_sources; } | ||
|
|
||
| private: | ||
| void rebuild_filepaths() | ||
| { | ||
| _filepaths.clear(); | ||
| _filepaths.reserve(_filepath_sources.size()); | ||
| for (auto const& source : _filepath_sources) { | ||
| _filepaths.push_back(source.path); | ||
| } | ||
| } | ||
|
|
||
| io_type _type = io_type::VOID; | ||
| size_t _num_sources = 0; | ||
| std::vector<filepath_source> _filepath_sources; | ||
| std::vector<std::string> _filepaths; | ||
|
mhaseeb123 marked this conversation as resolved.
|
||
| std::vector<cudf::host_span<std::byte const>> _host_buffers; | ||
| std::vector<cudf::device_span<std::byte const>> _device_buffers; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ | |
| #include <vector> | ||
|
|
||
| #ifdef CUDF_KVIKIO_REMOTE_IO | ||
| #include <kvikio/hdfs.hpp> | ||
| #include <kvikio/remote_handle.hpp> | ||
| #endif | ||
|
|
||
|
|
@@ -353,14 +354,43 @@ class user_datasource_wrapper : public datasource { | |
| datasource* const source; ///< A non-owning pointer to the user-implemented datasource | ||
| }; | ||
|
|
||
| #ifdef CUDF_KVIKIO_REMOTE_IO | ||
| /** | ||
| * @brief Infer the KvikIO remote endpoint type from a URL (no network I/O). | ||
| * | ||
| * Mirrors the order used by `kvikio::RemoteHandle::open()` in AUTO mode. | ||
| */ | ||
| kvikio::RemoteEndpointType infer_remote_endpoint_type(std::string const& url) | ||
|
TomAugspurger marked this conversation as resolved.
Outdated
|
||
| { | ||
| if (kvikio::S3Endpoint::is_url_valid(url)) { return kvikio::RemoteEndpointType::S3; } | ||
| if (kvikio::S3PublicEndpoint::is_url_valid(url)) { return kvikio::RemoteEndpointType::S3_PUBLIC; } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here a URL from public S3 would still be inferred as private S3.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be fair to say that there aren't really public and private URLs? Just URLs that happen to be anonymously accessible, and URLs that happen to require authorization, so they can't be disambiguated just by looking at the URL? I think something like rapidsai/kvikio#951 would solve this pretty cleanly: you define a chain of credentials to attempt when making some HTTP request. So long as anonymous access (no credentials) is included in the chain then things will succeed. For now, I think I'll remove the |
||
| if (kvikio::S3EndpointWithPresignedUrl::is_url_valid(url)) { | ||
| return kvikio::RemoteEndpointType::S3_PRESIGNED_URL; | ||
| } | ||
| if (kvikio::WebHdfsEndpoint::is_url_valid(url)) { return kvikio::RemoteEndpointType::WEBHDFS; } | ||
| if (kvikio::HttpEndpoint::is_url_valid(url)) { return kvikio::RemoteEndpointType::HTTP; } | ||
| return kvikio::RemoteEndpointType::HTTP; | ||
| } | ||
|
|
||
| kvikio::RemoteHandle open_remote_handle(char const* filepath, std::optional<std::size_t> known_size) | ||
|
TomAugspurger marked this conversation as resolved.
|
||
| { | ||
| if (known_size.has_value()) { | ||
| auto const endpoint_type = infer_remote_endpoint_type(filepath); | ||
| return kvikio::RemoteHandle::open(filepath, endpoint_type, std::nullopt, *known_size); | ||
| } | ||
| return kvikio::RemoteHandle::open(filepath); | ||
| } | ||
| #endif | ||
|
TomAugspurger marked this conversation as resolved.
Outdated
|
||
|
|
||
| #ifdef CUDF_KVIKIO_REMOTE_IO | ||
| /** | ||
| * @brief Remote file source backed by KvikIO, which handles S3 filepaths seamlessly. | ||
| */ | ||
| class remote_file_source : public kvikio_source<kvikio::RemoteHandle> { | ||
| public: | ||
| explicit remote_file_source(char const* filepath) | ||
| : kvikio_source{kvikio::RemoteHandle::open(filepath)} | ||
| explicit remote_file_source(char const* filepath, | ||
| std::optional<std::size_t> known_size = std::nullopt) | ||
| : kvikio_source{open_remote_handle(filepath, known_size)} | ||
| { | ||
| } | ||
|
|
||
|
|
@@ -397,7 +427,8 @@ class remote_file_source : public file_source { | |
|
|
||
| std::unique_ptr<datasource> datasource::create(std::string const& filepath, | ||
| size_t offset, | ||
| size_t max_size_estimate) | ||
| size_t max_size_estimate, | ||
| std::optional<std::size_t> known_size) | ||
| { | ||
| auto const use_memory_mapping = [] { | ||
| auto const policy = cudf::detail::getenv_or("LIBCUDF_MMAP_ENABLED", std::string{"OFF"}); | ||
|
|
@@ -410,7 +441,7 @@ std::unique_ptr<datasource> datasource::create(std::string const& filepath, | |
|
|
||
| if (remote_file_source::could_be_remote_url(filepath)) { | ||
| try { | ||
| return std::make_unique<remote_file_source>(filepath.c_str()); | ||
| return std::make_unique<remote_file_source>(filepath.c_str(), known_size); | ||
| } catch (std::exception const& ex) { | ||
| std::string redacted_msg; | ||
| try { | ||
|
|
@@ -450,7 +481,7 @@ std::unique_ptr<datasource> datasource::create(std::string const& filepath, | |
| // Create a remote file resource only when the pattern is found and replaced; otherwise, still | ||
| // create a local file resource | ||
| if (filepath != remote_file_path) { | ||
| return std::make_unique<remote_file_source>(remote_file_path.c_str()); | ||
| return std::make_unique<remote_file_source>(remote_file_path.c_str(), known_size); | ||
| } | ||
| } | ||
|
|
||
|
|
||
|
TomAugspurger marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| #include <cudf_test/base_fixture.hpp> | ||
| #include <cudf_test/column_utilities.hpp> | ||
| #include <cudf_test/column_wrapper.hpp> | ||
|
|
||
| #include <cudf/io/datasource.hpp> | ||
| #include <cudf/io/parquet.hpp> | ||
| #include <cudf/io/types.hpp> | ||
|
|
||
| #include <filesystem> | ||
|
|
||
| struct FilepathSourceTest : public cudf::test::BaseFixture {}; | ||
|
|
||
| TEST_F(FilepathSourceTest, StringConstructorsPopulateFilepathSources) | ||
| { | ||
| auto const single = cudf::io::source_info{"test.parquet"}; | ||
| ASSERT_EQ(single.filepath_sources().size(), 1); | ||
| EXPECT_EQ(single.filepath_sources().front().path, "test.parquet"); | ||
| EXPECT_FALSE(single.filepath_sources().front().size.has_value()); | ||
| EXPECT_EQ(single.filepaths(), std::vector<std::string>{"test.parquet"}); | ||
|
|
||
| auto const multi = cudf::io::source_info{std::vector<std::string>{"a.parquet", "b.parquet"}}; | ||
| ASSERT_EQ(multi.filepath_sources().size(), 2); | ||
| EXPECT_EQ(multi.filepaths().size(), 2); | ||
| EXPECT_FALSE(multi.filepath_sources()[1].size.has_value()); | ||
| } | ||
|
|
||
| TEST_F(FilepathSourceTest, FilepathSourceConstructorPreservesSize) | ||
| { | ||
| std::vector<cudf::io::filepath_source> sources{ | ||
| {"s3://bucket/object.parquet", 12345}, | ||
| {"https://example.com/data.parquet", std::nullopt}, | ||
| }; | ||
|
|
||
| auto const info = cudf::io::source_info{std::move(sources)}; | ||
| ASSERT_EQ(info.filepath_sources().size(), 2); | ||
| EXPECT_EQ(info.filepath_sources()[0].path, "s3://bucket/object.parquet"); | ||
| ASSERT_TRUE(info.filepath_sources()[0].size.has_value()); | ||
| EXPECT_EQ(info.filepath_sources()[0].size.value(), 12345); | ||
| EXPECT_FALSE(info.filepath_sources()[1].size.has_value()); | ||
| EXPECT_EQ(info.filepaths()[0], "s3://bucket/object.parquet"); | ||
| EXPECT_EQ(info.filepaths()[1], "https://example.com/data.parquet"); | ||
| } | ||
|
|
||
| TEST_F(FilepathSourceTest, KnownSizePlumbsThroughMakeDatasources) | ||
| { | ||
| auto const filepath = | ||
| (std::filesystem::temp_directory_path() / "filepath_source_test.parquet").string(); | ||
|
|
||
| auto col = cudf::test::fixed_width_column_wrapper<int32_t>{1, 2, 3}; | ||
| cudf::table_view const table{{col}}; | ||
|
|
||
| cudf::io::parquet_writer_options write_opts = | ||
| cudf::io::parquet_writer_options::builder(cudf::io::sink_info{filepath}, table); | ||
| cudf::io::write_parquet(write_opts); | ||
|
|
||
| auto const file_size = std::filesystem::file_size(filepath); | ||
| std::vector<cudf::io::filepath_source> sources{{filepath, file_size}}; | ||
| auto const source_info = cudf::io::source_info{std::move(sources)}; | ||
|
|
||
| auto datasources = cudf::io::make_datasources(source_info); | ||
| ASSERT_EQ(datasources.size(), 1); | ||
| EXPECT_EQ(datasources.front()->size(), file_size); | ||
|
|
||
| auto const read_opts = | ||
| cudf::io::parquet_reader_options::builder(source_info).column_names({"0"}).build(); | ||
| auto const result = cudf::io::read_parquet(read_opts); | ||
| CUDF_TEST_EXPECT_COLUMNS_EQUAL(col, result.tbl->get_column(0).view()); | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.