Lookup for vocab terms without hashmap in RDF reader

[filippodebortoli]

Leverages the vocab::Vocab enum to avoid building and passing around a Hashmap when deserializing named nodes in rdf::reader.

[phillord]

If I have understood the code correctly, the lookup from str to vocab enum happens through the vocab module. But, this ultimately backs onto the find_map iterator method. If this is correct, it is going to make the lookup linear wrt to the size of the vocab, rather than constant time which the HashMap lookup is, although avoiding the cost of building the HashMap in the first place.

It's nice that the code is cleaner, but I would worry that this would impact on performance.

[filippodebortoli]

If I have understood the code correctly, the lookup from str to vocab enum happens through the vocab module. But, this ultimately backs onto the find_map iterator method. If this is correct, it is going to make the lookup linear wrt to the size of the vocab, rather than constant time which the HashMap lookup is, although avoiding the cost of building the HashMap in the first place.

It's nice that the code is cleaner, but I would worry that this would impact on performance.

That is right.
Right now, Vocab::FromStr works as you described.
There is nothing preventing us from changing it to use a HashMap lookup.
This can be done e.g. by invoking Vocab::all() and collect it into a HashMap.
Or by using a more clever way to instantiate the HashMap - we could use std::sync::OnceLock (successor of lazy_static!) to create it similarly to what is done in enum_meta, except that the keys of the map are not discriminants, but strs.

Overall, I think that cleaning up the code is an important first step, and that changes can be done in vocab to ensure that this lookup does not impact too deeply on the parsing runtime. What do you think?

[phillord]

This looks good now? It is ready for merge?

[filippodebortoli]

This looks good now? It is ready for merge?

Yes, it is. I will merge now.