Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doc review! #1

Open
CAD97 opened this issue Dec 19, 2019 · 16 comments
Open

Doc review! #1

CAD97 opened this issue Dec 19, 2019 · 16 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@CAD97
Copy link
Owner

CAD97 commented Dec 19, 2019

I try to write great documentation, but I need others' eyes to make sure it's good. Check out the master documentation and put any feedback in this thread. (Actionable issues may of course still get their own issues.)

@CAD97 CAD97 added documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Dec 19, 2019
@danielhenrymantilla

This comment has been minimized.

@CAD97

This comment has been minimized.

@Ralith

This comment has been minimized.

@CAD97

This comment has been minimized.

@Ralith

This comment has been minimized.

@CAD97

This comment has been minimized.

@Ralith

This comment has been minimized.

@CAD97

This comment has been minimized.

@Ralith

This comment has been minimized.

@CAD97

This comment has been minimized.

@jplatte

This comment has been minimized.

@makoConstruct
Copy link

Thoughts after reading the slice_dst page: https://docs.rs/slice-dst/1.5.1/slice_dst/

I was very confused by the fact that the initializations of the Nodes in the second example don't have Arc::news. I missed the definition of Node and just noticed that they were not arcs and the diagram didn't make sense in light of that. (and on reflection, it wouldn't be possible to have a slice of them if they weren't Arc'd, as you really couldn't have an unsized type directly in a slice)

The example seems a bit odd, why would you want this functionality for a tree datatype? Trees are generally quite happy with indirected child lists.

I don't know what an obvious and natural example would look like, but I think it would narrow in on the amazing thing going on here if we used my application as the example:

I'm implementing CSR++, an updatable graph datastructure. It has a series of segments for its vertices. Each segment has a lock at the start and then a long series of vertices (length not known at compile-time). The segment pointers should be as well packed in the vertex array as possible, meaning that we want the length of the vertex array to be on the other side of the pointer, we don't want the pointer to be fat like rust pointers generally want to be, and then we want the lock on the segment to be with the vertex array, we want the vertices to be right there, not on the other side of yet another jump.

An aside, can slice-dst do that? Can it avoid having a fat pointer even though it's pointing to an unsized type? Does rust allow any way to point to an unsized type with a non-fat pointer?

@NobodyXu
Copy link

No, it can’t.

Though there is ThinArc in another crate triomphe, which allows you to store unsized data using one single pointer.

I recently submit a PR to triomphe to support another crate arc-swap, which can swap Arc automatically.

The PR is merged, but the repo owner hasn’t released a new version yet.

@CAD97
Copy link
Owner Author

CAD97 commented Aug 11, 2021

Actually, yes, this is a design point of slice-dst, or, rather, of erasable (which is also in this repo).

(The reason for the tree example is that these crates were originally written while hacking on Rowan, the syntax tree impl for rust-analyzer, with an eye specifically on minimizing allocations, but keeping tree-node-ownership (i.e. no vec+index handles), with the result being thin pointers to nodes which keep their children inline. Rowan also has the specific advantage of being (mostly) an immutable backing store.)

Specifically, using [erasable::Thin](https://docs.rs/erasable/1.2.1- /erasable/struct.Thin.html), you can wrap any pointer that implements erasable::ErasablePtr and points to a erasable::Erasable payload. Or, in plainer words, a smart pointer which you can into_raw into effectively void*C and recover back the fat pointer knowing only the (thin) pointer and type.

If you use SliceWithHeader, it contains the length inline, so it implements Erasable. As such, you could have struct Node(Thin<Arc<SliceWithHeader<SizedData, Node>>>); and Node would be a thin pointer to the length, header data, and (thin) child pointers in one allocation.

While the pointer-utils crates are more composable, triomphe is imo still more user-friendly, because slice-dst is very hard to use effectively. (Also, it requires first creating a Box of your type and then copying it to an Arc, since we use the std types and can't allocate an Arc directly, whereas since triomphe wrote their Arc they can skip the copy.) The main advantage of slice-dst and erasable is that you're using the standard containers.

Once feature(ptr_metadata) is stable, I believe I can make this process much easier. In the time after I originally released slice-dst, I've reconsidered the API surface multiple times, and aren't a big fan of it anymore. I'm actively experimenting with ptr_metadata and plan to eventually release indyn as a new approach to the custom DST type support.

With indyn down the road (the biggest added capability being support for non-slice DST tails, such as dyn Trait!), I don't have much motivation for improving slice-dst, due to its clunky (but functional!) API.

@CAD97
Copy link
Owner Author

CAD97 commented Aug 11, 2021

Side note on triomphe: there's an implicit reliance on storing &Header (thin) and using it to access &(Header + Slice) (fat). It's still unclear whether this is allowed by Rust's memory/borrowing model, and is currently disallowed by Stacked Borrows (miri) when tracking raw pointer validity more accurately (an additional flag on miri, due to the extra runtime cost). erasable (and slice-dst with erasable) suffer no such problem.

This is not unfixable, but will take a somewhat significant rewriting of triomphe's internals to fix (by using more thorough type erasure and/or more raw pointers) if deemed necessary. The general temperature of the UWG is that we want to support this (somewhat common) pattern, but it's not clear how to do so while retaining the "obviously correct" optimizations that Rust's strong aliasing xor mutability is supposed to allow for.

(If you know anything about how SB works: the problem is in reborrowing/retagging at function boundaries, which means references can only be used to access the amount of memory the type says it wants to, prohibiting the thin-to-fat expansion for references. erasable sidesteps this by always using type erased raw pointers for thin pointers.)

One other side note: erasable is suboptimal in that you have to use Thin<&Dst> to get a thin pointer, and &Dst is still a fat pointer. If we ever get extern type (avoiding the SB problem), I plan one more pointer-utils crate to allow library usage of extern type to provide "native"(ish) thin types to built-in fat DST tails, where &Dst is thin.

@makoConstruct
Copy link

further documentation feedback:

The use of Thin<Box<_>> in the example here https://docs.rs/erasable/1.2.1/erasable/struct.Thin.html might be making it unclear as to whether or how the type is being erased from the caller-facing API? My prior is that it isn't being erased there, but the API is called "erasable" so it introduces confusion. If it is erased, that should be made clearer too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants