Skip to content

Conversation

@vojtechtoman
Copy link
Contributor

@vojtechtoman vojtechtoman commented Mar 19, 2024

Which issue does this PR close?

Closes #9090.

What changes are included in this PR?

This is still work in progress. The PR introduces a few simple optimizations to to_timestamp (without format). On my local machine, the performance improvement in the benchmarks is about 15%.

Are these changes tested?

Yes, existing unit tests and benchmarks.

Are there any user-facing changes?

No.

@alamb alamb marked this pull request as draft March 19, 2024 14:13
@tustvold
Copy link
Contributor

I wonder if this could make use of the upstream parsers that are already highly optimised

https://docs.rs/arrow-cast/latest/arrow_cast/parse/index.html

@vojtechtoman
Copy link
Contributor Author

@tustvold the existing implementation was already using those (via string_to_timestamp_nanos). One thing I noticed early on while profiling is that the upstream parser can be optimized even further. This is why I copied a bunch of parsing code from upstream to this PR to see if there are any quick wins there (there are). Obviously, this should go into upstream eventually.

@tustvold
Copy link
Contributor

Heh, I thought the code looked familiar. Perhaps we could work on this upstream to avoid a load of duplicated code and make it more obvious what has changed?

@vojtechtoman
Copy link
Contributor Author

@tustvold Shall I open an issue in arrow-rs then?

@tustvold
Copy link
Contributor

Or just file a PR tbh, minor changes don't need issues

@vojtechtoman
Copy link
Contributor Author

@tustvold I have created apache/arrow-rs#5542

@github-actions
Copy link

Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale PR has not had any activity for some time label May 22, 2024
@github-actions github-actions bot closed this May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Stale PR has not had any activity for some time

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Optimize to_timestamp

2 participants