-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 all commits
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 & AFFILIATES. All rights reserved. | ||
| * 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, the IO backend may skip querying the remote server for file | ||
| * size at open time. | ||
| */ | ||
|
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 size at open time | ||
| }; | ||
|
|
||
| /** | ||
| * @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; | ||
|
|
||
|
TomAugspurger marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| * 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_test/table_utilities.hpp> | ||
|
|
||
| #include <cudf/io/datasource.hpp> | ||
| #include <cudf/io/parquet.hpp> | ||
| #include <cudf/io/types.hpp> | ||
|
|
||
| #include <filesystem> | ||
|
|
||
| auto const temp_env = static_cast<cudf::test::TempDirTestEnvironment*>( | ||
| ::testing::AddGlobalTestEnvironment(new cudf::test::TempDirTestEnvironment)); | ||
|
|
||
| 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 = temp_env->get_temp_filepath("KnownSize.parquet"); | ||
|
|
||
| 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).build(); | ||
| auto const result = cudf::io::read_parquet(read_opts); | ||
| CUDF_TEST_EXPECT_TABLES_EQUAL(table, result.tbl->view()); | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.