Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

filterx: native dict/list types #385

Merged
merged 8 commits into from
Jan 31, 2025
Merged

filterx: native dict/list types #385

merged 8 commits into from
Jan 31, 2025

Conversation

MrAnno
Copy link
Member

@MrAnno MrAnno commented Nov 19, 2024

No description provided.

@MrAnno MrAnno force-pushed the dict-list branch 5 times, most recently from a049f79 to 83c7160 Compare November 19, 2024 18:31
@bazsi
Copy link
Member

bazsi commented Dec 22, 2024

rebased against main.

@bazsi
Copy link
Member

bazsi commented Jan 22, 2025

I rebased this to current main and followed up the change of prototype of built-in simple functions.

@bazsi
Copy link
Member

bazsi commented Jan 22, 2025

This branch helps a lot in malloc() efficiency and json cloning, but the lookups take 2x as much than the JSON lookups.

With the JSON based build, get_subscript() takes 7.5% of the same run where with this branch (and the changing of the configuration from json() to dict()), it takes 14.4%

I was running a lot of tests, so this might not be apples-to-apples completely, but that would be the reason for not improving performance that much.

So I would definitely merge this, it is so much better on all other aspects than the JSON based build, than I am inclined to ditch that (and protobuf) completely and use just one internal/optimized representation and convert on input/output.

@MrAnno MrAnno marked this pull request as ready for review January 28, 2025 13:38
@MrAnno
Copy link
Member Author

MrAnno commented Jan 28, 2025

I'm moving out this from draft as we discussed that this will be merged and reworked.

@bazsi bazsi merged commit 6c57a1f into axoflow:main Jan 31, 2025
22 checks passed
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