rapidsai / cudf

cuDF - GPU DataFrame Library
https://docs.rapids.ai/api/cudf/stable/
Apache License 2.0
8.47k stars 907 forks source link

[FEA] Replace internal usage of std::string with std::string_view #15907

Open vyasr opened 5 months ago

vyasr commented 5 months ago

Is your feature request related to a problem? Please describe. There are currently a number of libcudf functions that accept a std::string argument when they don't need to own the string but could simply use a view. Others take a const ref to a std::string, explicitly indicating that they don't require ownership. These include:

There may be others as well.

Describe the solution you'd like We should do a thorough audit of std::string usage in libcudf to ensure that we are properly using std::string_view instead where appropriate.

Additional context A subset of this request was originally documented in https://github.com/rapidsai/cudf/issues/14413, which indicates that some of these changes may be nontrivial to implement.

karthikeyann commented 5 months ago

Line 386 in json.hpp is std::map of std::string. string_view can not be used here because the string_view's underlying string may get invalidated and the string_view will still point to destroyed/modified object memory.

karthikeyann commented 5 months ago

source_info constructor could be updated to take string_view https://github.com/rapidsai/cudf/blob/branch-24.08/cpp/include/cudf/io/types.hpp#L338 It's common for many readers