Repository navigation
Performance bottleneck in VOTable parsing with BINARY2 serialization (converters.py) #18442
Description
What is the problem this feature will solve?
We’re seeing relatively slow performance when parsing large VOTables using the BINARY2 serialization format. This is somewhat expected due to the nature of the language rather than anything wrong with the architecture of the parsing code. Profiling results from py-spy show a significant portion of time is spent in astropy.io.votable.converters.binparse. Here’s an excerpt of the profiler output:
py-spy top -- python test.py
Collecting samples from 'python test.py' (python v3.12.7)
Total Samples 1600
GIL: 100.00%, Active: 100.00%, Threads: 1
%Own %Total OwnTime TotalTime Function (filename)
54.00% 57.00% 7.69s 8.26s binparse (astropy/io/votable/converters.py)
24.00% 100.00% 3.68s 14.66s parsebinary (astropy/io/votable/tree.py)
4.00% 4.00% 0.940s 0.940s bitarray_to_bool (astropy/io/votable/converters.py)
8.00% 8.00% 0.890s 0.910s setitem (numpy/ma/core.py)
0.00% 0.00% 0.470s 0.670s getbinary_data_stream (astropy/io/votable/tree.py)
2.00% 2.00% 0.450s 0.450s careful_read (astropy/io/votable/tree.py)
0.00% 0.00% 0.430s 0.430s new (numpy/_core/records.py)
As shown, the hot path is dominated by the binparse function in converters.py.
Suggestion
My suggestion is to migrate the logic of the binary2 converters to Cython.
As a proof of concept, I've prototyped a Cython version of the critical path and observed a ~50% performance improvement on large VOTables.
Specifically, I tested with a a VOTable with 50000 rows and aprox. 1000 columns (~50 million cells).
With the current implementation, this takes ~65 seconds to parse, whereas with the Cython-ized version it took about ~30 seconds.
Here is the prototype PR with the changes:
https://github.com/stvoutsin/astropy/pull/1/files
Is this a reasonable approach?
If so, I'd be happy to contribute this upstream, or perhaps leave it to someone with more Cython expertise?
Describe the desired outcome
Improved performance for parsing of Binary2 VOTables.
Activity
mhvk commentedon Jul 17, 2025
Interesting that there is so much to gain. Could you be a bit more specific about which data types are slow and fast, though? E.g., for doubles, I think currently
np.frombufferis used and I am surprised that would be so much slower than your cython code.More generally, I wonder if better use of numpy routines might give comparable speed-up, without sacrificing the ease of python.
mhvk commentedon Jul 17, 2025
p.s. One suggestion would be to add relevant benchmarks to https://github.com/astropy/astropy-benchmarks -- that way it will be easier to show what performance improvements one really gets. 50% is definitely worth having!
pllim commentedon Jul 17, 2025
As for the PR, since you already did the work, could always open a draft PR here and see if the CI pass and also try run a relative benchmark. Up to you. Thanks, all!
stvoutsin commentedon Jul 22, 2025
Thanks for the feedback!
Adding benchmarks was quite useful in identifying the improvements. What I see with the initial benchmark that I've added a PR for is 53% speedup on numeric tables, 43-47% on mixed data.
I think the slowness might not be just
np.frombufferitself, but the overhead around it, like for example creating intermediate arrays (small operations which add up with large tables) which we avoid in the Cython version.I initially explored improving performance within numpy, but the challenge is that we're processing mixed row structures (strings, arrays, different types) sequentially and I'm not sure how you can property vectorize because of the existence of variable-length fields.
I can PR the Cython prototype, but happy to try out other suggestions/ideas as well.