search

huggingface / safetensors

Simple, safe way to store and distribute tensors
https://huggingface.co/docs/safetensors
Apache License 2.0
2.72k stars 182 forks source link

Question about parallelism/rayon #15

Closed mishig25 closed 7 months ago

mishig25 commented 1 year ago

Questions regarding parallelism:

  1. If I'm not mistaken, both tensor serialization & deserialization operations should be parallelizeable. Is this assumption correct? For example, I was thinking this line below can be replaced with par_extend https://github.com/huggingface/safetensors/blob/86e356fed6b75d4b0a06ee446ab94f1317eafc2e/safetensors/src/tensor.rs#L59
  2. If point 1 is valid, does it make sense to do so? Or is it not worth it (i.e. speed up might not be great OR we don't want to introduce dependency on rayon)?

Thanks so much! 🤗 cc: @Narsil

Narsil commented 1 year ago

Hi @mishig25 ,

Have you tried it ? I'm curious to see the results.

Since buffer is mutable I expect this will not be trivially parallelizable. It's probably technically doable in this function since all buffer sizes are known at this point.

For writing to disk serialize_file (probably the most used code path) I anticipate, the limiting factor would be the actual disk speeds not CPUs, and random disk access is I think worse than serialized one even for SSDs.

rayon is a big dependency and I wouldn't add it without solid reason. One of the biggest merits of this lib is the smallish size and very low dependencies (currently only serde and serde_json). But if it's 10x faster then yes let's do it.

github-actions[bot] commented 7 months ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.