-
Notifications
You must be signed in to change notification settings - Fork 73
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Second batch of performance improvements
- Loading branch information
Showing
12 changed files
with
127 additions
and
102 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
554f5d5
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.
@dehesa just curious, does this result in measurable performance improvements? Does the compiler not optimize out the retain / release calls from
withExtendedLifetime
andpassUnretained
calls?554f5d5
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.
Hi @JackYoustra,
At the time I was introducing this I was seeing in Instruments a big chunk of of the time was spent in retain/release (I don't remember on the top of my head, but I believe it was around 35%). Therefore, the compiler was not optimizing the reference lifecycle functions even for simple test samples (with large CSVs).
My guess was that, due to the usage of different modules (
CodableCSV
and the caller module), the compiler was unable to optimize this way. So, I wanted to slowly try to remove any reference type usage and measure it through strict benchmarking. That is why the next point in the Roadmap is "Benchmarking".Sadly, I haven't had the time to continue with that effort. As you can see there is still many reference type usage and the benchmarks aren't done 🤷♂️
554f5d5
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.