[libcxx-commits] [PATCH] D56692: More calendar bits for <chrono>

[Marshall Clow via Phabricator via libcxx-commits]

mclow.lists added inline comments.


================
Comment at: include/chrono:2748
+private:
+    string __name;
+    string __target;
----------------
mclow.lists wrote:
> EricWF wrote:
> > I think there is a better representation for this.
> > 
> > I have a couple of ideas, but it depends on what the implementation for `time_zone` and `tzdb` look like.
> > For example, the `target()` string should always correspond to a `time_zone` with the same name. We should be able to re-use that string instead of allocating another.
> > 
> > And if we can't re-use strings that we already store, I was thinking something like this:
> > ```
> > struct Link2 {
> >     Link2(std::string_view xname, std::string_view xtarget) : name_len(xname.size()), target_len(xtarget.size()) {
> >         buff.reset(new char[name_len + target_len + 2] );
> >         memcpy(buff.get(), xname.data(), name_len + 1);
> >         memcpy(buff.get() + xname.size() + 1, xtarget.data(), target_len + 1);
> >     }
> >     
> >     std::string_view name() { return std::string_view(buff.get(), name_len); }
> >     std::string_view target() { return std::string_view(buff.get() + name_len + 1, target_len); }
> >     std::unique_ptr<char[]> buff;
> >     unsigned short name_len, target_len;
> > };
> > ```
> > 
> > This gives us one allocation and a object size of 16 bytes (as opposed to 64), a less expensive move operation, and more consistent invalidation semantics on the `string_view`.
> You know what I like better? store a `string` and a `string_view` and construct from a `string` + `time_zone`. Note that an overwhelming majority of the names will fit in the short string optimization.
> and more consistent invalidation semantics on the `string_view`.

The `string_view` should never be invalidated. Once the vector of links is created in the tzdb, then it never moves/changes/gets deallocated. Before the tzdb is created, no one calls `name` or `target`.

As for the allocations, as I said before, "an overwhelming majority of the names will fit in the short string optimization." (~560 out of ~590 across both zones and links). So most of the time, using strings involves NO allocations.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56692/new/

https://reviews.llvm.org/D56692