beef icon indicating copy to clipboard operation
beef copied to clipboard

Some actual proper design for internals

Open maciejhirsz opened this issue 4 years ago • 4 comments

Messing around between the fat pointer and thin pointer is quite a lot of work, and there is quite some deep coupling between generic::Cow, the Beef trait and the Capacity. Capacity is also a poor name for what the trait is doing now.

A reasonable logical split would be:

  • generic::Cow implements the API surface, methods like borrow, owned and into_owned, as well as all the traits one expects.
  • Beef converts borrowed and owned values into parts (pointer, len, cap) and back.
  • Rename Capacity to something like Container, have it store (pointer, len, cap) from Beef internally so that the particulars of how capacity is handled doesn't leak out to other abstractions.

maciejhirsz avatar Mar 18 '20 23:03 maciejhirsz

Is there a reason why it needs to be (pointer, len, cap) and not (pointer, cap, len) for Lean? Is it related to performance?

pickfire avatar Mar 27 '20 02:03 pickfire

No, the Container here would be able to arrange stuff in whatever order it wants. Currently lean version is (ptr, len) with capacity put into higher 32 bits of len, so that obtaining a reference can be done by just masking those bits.

maciejhirsz avatar Mar 27 '20 07:03 maciejhirsz

Yeah, regarding the current design, I was asking why capacity were put into the higher 32 bits of length rather than length were put into the higher 32 bits of capacity? Isn't capacity being used more since it also stores the tag when capacity is 0?

pickfire avatar Mar 28 '20 02:03 pickfire

No, capacity is only used:

  • When you call into_owned.
  • When the Cow is cloned.
  • When the Cow is dropped.

Most of what you want to do with a Cow is to read from it, which means creating a &ref (via Deref or Borrow). With the pointer and length in the same position for owned and borrowed values, getting a &ref is, unlike std implementation, branch free.

maciejhirsz avatar Mar 28 '20 06:03 maciejhirsz