Few benchmarks for predicate evaluation #291
Merged
Conversation
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
…e from the package-data-model.
…on-functionality-instead
fix(patch): [sc-3840] Refer branch instead of commit hash.
…ndation-functionality-instead fix(patch): [sc-3840] Refer dependencies using branches to avoid SPM complaining.
…ndation-functionality-instead-2 fix(patch): Rename ordo/swift-foundation to ordo/package-swift-foundation.
…ndation-functionality-instead-3 Bump dependency version.
…ndation-functionality-instead-4 fix(patch): Bump dependency version
…ndation-functionality-instead-5 Bump dependency version.
FranzBusch
reviewed
Oct 11, 2023
…ft-foundation into feature/predicate-benchmark
|
@ser-0xff I think the test needs to use existentials as we do for transactions internally, the current benchmark does not show the same key path problem we see with a protocol type wrapper. |
|
Alright, let me look into. |
hassila
approved these changes
Oct 12, 2023
iCharlesHu
reviewed
Oct 12, 2023
Add standard project header
|
@swift-ci please test |
|
Thanks again for posting this, this looks super exciting and will definitely help us expand on our performance testing of this package going forward. We're still looking into this and will get back to you on being able to merge this soon! |
jmschonfeld
reviewed
Nov 3, 2023
|
Addressed PR feedback in 80a84c3 - also added ARC output by default (even though some implicit retains are not necessarily counted currently as some hooks in the runtime are missing, see swiftlang/swift#64636 - it's still useful to see the relative usage of ARC traffic between different test scenarios even if retain/releases doesn't square out exactly.) Current output (throughput is in K): |
jmschonfeld
reviewed
Nov 6, 2023
…le check for macOS 14
… tests (the test number is mostly useful for filtering with --filter when wanting to run a specific test)
…r for manual workflows, e.g. swift package benchmark --filter predicateKeypathPropertyCondition
|
Ok, simplified the naming of the test to make runs for specific tests easier, e.g. swift package benchmark --filter predicateKeypathPropertyConditionAlso added a simple variadic test to see what the impact would be when going from 1 -> 2 at least (seems to be 30% or so). Definitely room for extending that later. |
…till get enough samples and stable results
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
jmschonfeld
approved these changes
Nov 7, 2023
Sounds great, thanks - if you are interested in adding the benchmark suite into CI for regression checking in the future, I would suggest having a look at how swift-nio does it. Happy to help out if you run into any questions on the usage of package-benchmark if looking at wider adoption (and we'll provide new PR:s for any performance issues we run into, currently we are focused on adopting Predicates though). |
Good to know, thanks! @iCharlesHu and I were discussing this earlier, so Charles it looks like that'd be a good place to start.
Sounds great! Definitely let us know if you hit other performance bottlenecks and we can definitely take a look to see if we can come up with some solutions! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Predicate matching performance is 10x slower over custom implementation, this PR adds benchmark plugin to get a baseline performance and to help future Predicates development.
Benchmark is added in the same fashion as for swift-nio and swift-certifcates.
There are 5 benchmarks.
The most common case for our product is more than one condition, and we widely use computed properties, so if we will use more than 3 conditions then we will have throughout less than 600K/sec, which not too much for us.
All benchmarks can be executed with
swift package benchmark --scalein the project 'Benchmarks' directory.We observe
swift_getAtKeyPathexecution takes significant time relative to whole execution time.The issue can be reproduced with a nested computed properties in benchmark (4) and (5). A single benchmark can be executed with command
swift package benchmark --scale --filter "Predicate #5 - 3 KeyPath nested computed property conditions"Also we observe that predicate evaluation entails few malloc calls, what is also possibly the cause of poor performance.
Each benchmark report a set of information, the most interesting for now is a 'Throughput' line, which shows how many evaluations per second can be executed.
To run with profile in Xcode:
Running benchmarks in Xcode and using Instruments for profiling benchmarks
Profiling benchmarks or building the benchmarks in release mode in Xcode with jemalloc is currently not supported (as Xcode currently doesn’t support interposition of the malloc library) and requires disabling jemalloc.
Make sure Xcode is closed and then open it from the CLI with the BENCHMARK_DISABLE_JEMALLOC environment variable set e.g.:
This will disable the jemalloc dependency and you can simply build in Xcode for profiling and use Instruments as normal - including signpost information for the benchmark run.