google-cloud-cpp icon indicating copy to clipboard operation
google-cloud-cpp copied to clipboard

Optimize initial `OptionSpan`

Open coryan opened this issue 2 years ago • 2 comments

Every request in a *Client starts with something like:

 google::cloud::internal::OptionsSpan span(std::move(options), options_);

The options_ member variable never changes value. We should avoid the implicit copy from these options. If we can hold them as a std::shared_ptr<Options const> then maybe ImmutableOptions can become a class that holds two Options, one by value (initialized via move) and a std::shared_ptr<> with the remaining values.

Something like:

class internal::OptionsImpl { // Similar to the Options class today
public:
  template <typename T> auto get() const { ... }
  // has<T>() and get<T>() rolled into a single operation.
  template <typename T> std::pair<bool, typename T::Type*> find() const { ... }
};

class internal::ImmutableOptions {
public:
 template<typename T> auto get() const {
   auto [has, value] = impl_.find<T>();
   if (has) return *value;
   return fallback_.get<T>();
 }

 // No set or unset member functions.

private:
  OptionsImpl impl_;
  std::shared_ptr<OptionsImpl const> fallback_;
};

class Options {

private:
  OptionsImpl impl_;
};

coryan avatar Aug 18 '23 17:08 coryan

This is blocked on #12359. And maybe we can also change the mocks (in a backwards-compatible way) and remove the need for this.

coryan avatar Feb 14 '24 20:02 coryan