Skip to content

change result #148

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

Open
wants to merge 3 commits into
base: result-as-tuple
Choose a base branch
from
Open

Conversation

bymoye
Copy link
Contributor

@bymoye bymoye commented Jul 1, 2025

No description provided.

@bymoye
Copy link
Contributor Author

bymoye commented Jul 1, 2025

I also modified one thing in the code (I just did some simple research):

.call((raw_bytes_data.to_vec(),), None)?
to:

.call1((PyBytes::new(py, raw_bytes_data),))?
Reason:
This reduces data copying from 2 operations (Rust Vec creation + Python internal copy) to 1 operation (direct copy to Python memory).

@bymoye
Copy link
Contributor Author

bymoye commented Jul 1, 2025

@chandr-andr ,In 8cc74de, I made further optimizations to improve performance. I only conducted simple tests with pytest, no benchmarking. Can you run some tests? If there's a performance regression, feel free to roll back the code.
It is best to increase the memory usage test!

@chandr-andr
Copy link
Member

@bymoye Thank you very much for refactoring, it looks awesome.
I cannot run benchmarks right now cuz I have to work (unfortunately).

Will run them in the evening

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