ksahlin / strobealign

Aligns short reads using dynamic seed size with strobemers
MIT License
142 stars 17 forks source link

Use string views instead of strings when extracting query randstrobes #281

Closed althonos closed 1 year ago

althonos commented 1 year ago

Hi!

I am wrapping strobealign and using it with another C library for processing reads, and my reads are stored in raw C-strings for ABI compatibility with the rest of the code. However, this means that I need to copy the reads to a std::string at every query, which can become expensive.

This PR updates query_randstrobes so that it uses a std::string_view rather than a std::string, avoiding unneeded copies. std::string_view was added in C++17, which is the minimum required version anyway. std::string_view can be obtained cheaply (without copy) from both a C++ std::string and an raw const char* pointer.

marcelm commented 1 year ago

Thanks; being able to use string_view was one of the reasons why I bumped the required C++ from 14 to 17 (although we haven’t started using it). It’s good to see that it also helps with making strobealign easier to use from C.

I’ll have a closer look next week. If you want, you could investigate the failing Python bindings, but I can also look into that if not.

marcelm commented 1 year ago

Thanks again, also for fixing the Python bindings so quickly.

In case you have other suggestions for making wrapping strobealign easier, please let us know!

althonos commented 1 year ago

Sure thing, thanks for the responsiveness! :+1: