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

fix(accept): schecule fuzzy.access using uv.new_work #522

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

stefanboca
Copy link
Collaborator

No description provided.

@stefanboca stefanboca force-pushed the stefanboca/push-ltrtynkllkvp branch 2 times, most recently from 7ae8bb3 to b32446e Compare December 12, 2024 23:46
Base automatically changed from stefanboca/push-qmwkvvqwlpqq to main December 12, 2024 23:46
@stefanboca stefanboca force-pushed the stefanboca/push-ltrtynkllkvp branch from b32446e to b503f34 Compare December 12, 2024 23:48
@Saghen
Copy link
Owner

Saghen commented Dec 13, 2024

Small nit, i think ideally the uv.new_work would be inside the access function so consumers don't have to worry about it

@stefanboca
Copy link
Collaborator Author

stefanboca commented Dec 13, 2024

Good idea, I'll make the change.

This is actually not working currently, because tables (item) can't be passed as thread args. I didn't realize this at first because everything was running in a context where errors were silent. Maybe the item needs to be vim.mpack serialized/deserialized? LuaJIT also has string.buffer.{encode,decode}

@Saghen
Copy link
Owner

Saghen commented Dec 13, 2024

You might want to use the built in message pack API to serialize it before sending

@stefanboca
Copy link
Collaborator Author

Just did a quick benchmark: for a roundtrip encode/decode on a typical item, vim.mpack takes ~0.02ms, while string.buffer takes ~0.002ms. The cost is so small that IMO it's not worth dynamically detecting if string.buffer is available.

@stefanboca stefanboca force-pushed the stefanboca/push-ltrtynkllkvp branch from b503f34 to 169cc42 Compare December 13, 2024 00:39
@stefanboca stefanboca force-pushed the stefanboca/push-ltrtynkllkvp branch from 169cc42 to 92112fe Compare December 13, 2024 00:40
@Saghen Saghen merged commit f66f19c into main Dec 13, 2024
2 checks passed
@Saghen Saghen deleted the stefanboca/push-ltrtynkllkvp branch December 13, 2024 01:02
@Saghen
Copy link
Owner

Saghen commented Dec 13, 2024

Looks great, thank you!

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