-
Notifications
You must be signed in to change notification settings - Fork 320
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
Parsing speed improvements #4297
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks so much for doing this!
return calendar | ||
}() | ||
|
||
private func toDateTryFastParsing() -> Date? { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dumb little nitpicks that you can totally disregard:
private
here is redundant in that the extension is already private, we try not to repeat
We usually add line breaks after extension definitions and before they end
See examples in the style guide
Also, given that the method already returns an optional type, it feels like the try
part is also redundant, I'd maybe go with toDateFastParse
Again, I don't feel particularly strongly about any of these tbh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Please let me know if there is anything else you wish me to change.
Checklist
purchases-android
and hybridsMotivation
I was scrolling through your code and saw
Which suggested that you care about performance of the parser. Since I wanted to improve my profiling skills here we are :)
Issue: #4006
Description
Improved performance of parsing receipt (which is mostly parsing of dates).
From profiler I saw that most of the time is spent in
ArraySlice.toDate()
.By removing
dateString = String
and using system Calendar I was able to sigificently speed up the process.As fallback I left previous
ISO8601DateFormatter.default.date
Example results on iPhone 8
Date
mach_absolute_time
Cycles
INST_ALL
L1D_TLB_MISS
I've also added some additional test cases based on example receipts I was able to find through internet.