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: Pass slices around instead of Vec's #27

Merged
merged 2 commits into from
Aug 5, 2023
Merged

fix: Pass slices around instead of Vec's #27

merged 2 commits into from
Aug 5, 2023

Conversation

Uzaaft
Copy link
Contributor

@Uzaaft Uzaaft commented Aug 5, 2023

No description provided.

@Uzaaft
Copy link
Contributor Author

Uzaaft commented Aug 5, 2023

This had a minor perf increase for the full backtest locally, ranging from 1-2%. Screenshot below.
image

@calumrussell calumrussell merged commit 91c4d66 into calumrussell:main Aug 5, 2023
1 check passed
@calumrussell
Copy link
Owner

Nice! Appreciate your time and effort. I will have to look out for this in future because it clearly makes a big difference (and slice usage is more idiomatic I believe).

@Uzaaft
Copy link
Contributor Author

Uzaaft commented Aug 5, 2023

Nice! Appreciate your time and effort. I will have to look out for this in future because it clearly makes a big difference (and slice usage is more idiomatic I believe).

Yeah, seems like that.
I found your usage of returning &Vec<T> confusing. If you want to return a reference, I'd use &[T], but that causes alot of breaking changes, especially with data held by the structs and lifetimes

@calumrussell
Copy link
Owner

Nice! Appreciate your time and effort. I will have to look out for this in future because it clearly makes a big difference (and slice usage is more idiomatic I believe).

Yeah, seems like that. I found your usage of returning &Vec<T> confusing. If you want to return a reference, I'd use &[T], but that causes alot of breaking changes, especially with data held by the structs and lifetimes

I can see the sections you are referring to. Yes, it seems like the DataSource trait could have perf implications, and that would be called multiple times in the main loop. I don't think I am hitting this but I will see what the implications of changing that would be because it isn't ideal.

I am still trying to improve my Rust so appreciate the further explanation.

@calumrussell calumrussell mentioned this pull request Aug 5, 2023
@Uzaaft
Copy link
Contributor Author

Uzaaft commented Aug 6, 2023

We could perhaps use the key takeways from this video?
https://www.youtube.com/watch?v=A4cKi7PTJSs

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