Skip to content

Conversation

@Nicceboy
Copy link
Contributor

@Nicceboy Nicceboy commented Apr 6, 2025

I am still doing some benchmarking and optimizations, and it seems that TagTree::tag_contains takes a lot time when there are many Choice types in the overall type.

It seems to call itself recursively at least once no matter what it contains.
This adds ups since when especially decoding Choice variant that is the last in order, tag_contains can be called 1 + amount of variants or even more.

So I have tried to replace TagTree with simple &'static [Tag] whenever possible, including in Choice trait, and there is a pretty significant performance boost available.

I still have to test if there are some side effects what current tests are not showing.

image

At least current codec implementations just want to know whether tag exists with VARIANTS/EXTENDED_VARIANTS so maybe this can be changed.

@Nicceboy Nicceboy marked this pull request as draft April 6, 2025 12:06
@XAMPPRocky
Copy link
Collaborator

At least current codec implementations just want to know whether tag exists with VARIANTS/EXTENDED_VARIANTS so maybe this can be changed.

Doesn't this affect SET implementations though since they need to be able figure out what the tag is for each component and there could be multiple nested choice types?

@Nicceboy
Copy link
Contributor Author

Nicceboy commented Apr 6, 2025

For nested choices it currently uses conditionally TagTree::tag_contains call, which should be enough for that, and reduces the overhead from everywhere else. This happens on derived code rather than on codec level. But that needs more tests I think.

@Nicceboy
Copy link
Contributor Author

Also note - if &'static [Tag] is valid and can be used, in many cases tags can also be sorted and we could use binary_search instead. On few variants there is no speedup but if someone uses extremely many variants, it would be visible. Premature optimization?

@Nicceboy Nicceboy force-pushed the choice-variant-tag-optimization branch from 3652b67 to e443c84 Compare August 1, 2025 07:07
@Nicceboy Nicceboy force-pushed the choice-variant-tag-optimization branch from e443c84 to dcc6244 Compare August 11, 2025 16:54
@XAMPPRocky
Copy link
Collaborator

@Nicceboy friendly check in, is this still something you're interested in pursuing?

@Nicceboy
Copy link
Contributor Author

Nicceboy commented Oct 8, 2025

This is still on the list, I just have been a bit busy. Maybe we can do binary search part later.

@Nicceboy Nicceboy force-pushed the choice-variant-tag-optimization branch from dcc6244 to cf4325e Compare October 24, 2025 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants