[Core] Replace read-only string parameters with string_view on critical path

[tianyi-ge]

Description

Replace const std::string & with std::string_view to avoid temporary string for a string literal or substring

Related issues

Closes #59887

Additional information

protobuf ParseFromString only accepts std::string.

ray/src/ray/core_worker/actor_handle.cc

Lines 64 to 68 in c49b666

	rpc::ActorHandle CreateInnerActorHandleFromString(const std::string &serialized) {
	rpc::ActorHandle inner;
	inner.ParseFromString(serialized);
	return inner;
	}

[gemini-code-assist]

Code Review

This pull request replaces const std::string& with std::string_view for read-only string parameters on performance-critical paths. This is a good optimization to avoid unnecessary string allocations. The changes are applied consistently across several core worker components. My review includes suggestions to further optimize the use of std::string_view with protobuf setters to prevent the creation of temporary std::string objects, thereby fully realizing the performance benefits of this change.

[gemini-code-assist]

To fully realize the performance benefits of std::string_view, the protobuf setter in the function body should be updated. message_->set_name(name); will create a temporary std::string. To avoid this, you should use the (const char*, size_t) overload: message_->set_name(name.data(), name.size());.

[gemini-code-assist]

To fully leverage std::string_view and avoid temporary std::string allocations, please use the (const char*, size_t) overload for the protobuf setters in this function's body. For example: message_->set_debugger_breakpoint(debugger_breakpoint.data(), debugger_breakpoint.size());. This applies to call_site and concurrency_group_name as well.

[gemini-code-assist]

To avoid temporary std::string creation when setting the protobuf field, please use the (const char*, size_t) overload: message_->set_serialized_retry_exception_allowlist(serialized_retry_exception_allowlist.data(), serialized_retry_exception_allowlist.size());

[gemini-code-assist]

To avoid temporary std::string creation, please use the (const char*, size_t) overload for the protobuf setter: actor_creation_spec->set_serialized_actor_handle(serialized_actor_handle.data(), serialized_actor_handle.size());

[gemini-code-assist]

To avoid temporary std::string creation, please use the (const char*, size_t) overload for the protobuf setter: actor_creation_spec->set_extension_data(extension_data.data(), extension_data.size());

[gemini-code-assist]

To avoid temporary std::string creation, please use the (const char*, size_t) overload for the protobuf setter: message_->set_serialized_retry_exception_allowlist(serialized_retry_exception_allowlist.data(), serialized_retry_exception_allowlist.size());

[gemini-code-assist]

To fully leverage std::string_view and avoid temporary std::string allocations, please use the (const char*, size_t) overload for the protobuf setters in this function's body. For example, inner.set_extension_data(extension_data.data(), extension_data.size());. This applies to name and ray_namespace as well.

[gemini-code-assist]

To avoid temporary std::string allocations with protobuf setters, please use the (const char*, size_t) overload. For example: task_log_info.set_stdout_file(stdout_path.data(), stdout_path.size());. This also applies to stderr_path.

[edoakes]

@israbbani @dayshah PTAL

[israbbani]

@tianyi-ge. Thanks for the contribution. I've started some discussion on the original issue. #59887. I'll hold off on reviewing this until we finish the discussion there.