lo48576 / iri-string

String types for URIs/IRIs.
Apache License 2.0
17 stars 3 forks source link

Reference-Counted versions #20

Closed damooo closed 2 years ago

damooo commented 2 years ago

As iri types are immutable, It will enable allocation-less cheap clones if there are versions backed by reference counted invariants, like Arc<str>, and Rc<str>. It will be good performance improvement for iri intensive applications like those dealing with rdf graphs.

lo48576 commented 2 years ago

You can already do that, while it is hard to know it from the doc... The borrowed and owned string types in this crate try to mimic string types in std like str, Path, OsStr..., so you can use them with Box, Cow, Arc, and Rc!

Example: From<&'_ RiStr<S>> for Arc<RiStr<S>>: https://docs.rs/iri-string/0.5.2/iri_string/types/struct.RiStr.html#impl-From%3C%26%27_%20RiStr%3CS%3E%3E

You can do like the below:

#[cfg(test)]
mod tests {
    use iri_string::types::{IriStr, IriString};
    use std::sync::Arc;

    #[test]
    fn string_to_arc() {
        let s = IriString::try_from("https://example.com/").unwrap();

        // `Arc::from(s)` will make `Arc<IriString>`, but usually it is not what we want.
        let arc = Arc::<IriStr>::from(s.as_slice());
        //let arc: Arc<IriStr> = s.as_slice().into(); // Also OK.
        let arc2 = Arc::clone(&arc);

        drop(arc);
        assert_eq!(*arc2, "https://example.com/");
    }

    #[test]
    fn str_to_arc() {
        let s = IriStr::new("https://example.com/").unwrap();

        // Oh no, `Arc::from(s)` is ambiguous... `Arc::<&IriStr>::from` is another candidate.
        let arc = Arc::<IriStr>::from(s);
        //let arc: Arc<IriStr> = s.into(); // Also OK.
        let arc2 = Arc::clone(&arc);

        drop(arc);
        assert_eq!(*arc2, "https://example.com/");
    }
}

Type annotation like Arc::<IriStr>::from is annoying, but it is also necessary for std string (Arc::from("hello") is ambiguous), so I believe I can do nothing on my end.

I'll consider improving docs to explain how to convert to wrapped string types. Thanks for reporting!

lo48576 commented 2 years ago

Note that From<IriString> for Arc<IriStr> and similar conversions are not implemented at least for now, since it is actually reallocating memory.

As you can see in the From<String> for Arc<str> implementation, it is actually Arc::from(&v[..]), and does not reuse already-allocated memory. So, Arc::<IriStr>::from(iri_string) cannot be more efficient than Arc::<IriStr>::from(iri_string.as_slice()) and then drop(iri_string).

Of course I can implement such direct conversions, but I'm not sure it is worth doing, because it hides memory allocation cost in return for reducing only 11 characters .as_slice().

If you have any opinions, ideas, or belief, feel free to tell me.

damooo commented 2 years ago

@lo48576 , thanks for detailed answer. Will be using invariants wrapped in arcs as you specified instead.

The original request was to have a type that wraps an Arc<str>, not for wrapping iri in Arc. That is, something like:

pub struct RiArcStr(Arc<str>);

which is backed by Arc<str>, and can be used as cheaply clonable iri.

lo48576 commented 2 years ago

I'll consider improving docs to explain how to convert to wrapped string types.

Added examples to the doc (https://docs.rs/iri-string/0.5.3/iri_string/types/index.html#wrapped-string-types). Closing as resolved.