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

Add alloc counter benchmark for result erasing maps. #1858

Merged
merged 1 commit into from May 25, 2021

Conversation

Lukasa
Copy link
Contributor

@Lukasa Lukasa commented May 25, 2021

Motivation:

Peter thinks that result-erasing maps should not allocate, and we have
special code paths in the code to try to make Void -> Void maps not
allocate. Sadly, both code paths currently do allocate.

Per our rules for not trying to make optimizations without data, we
should start measuing these closures so we can make optimizations.

Modifications:

  • Added an alloc couter test for result-erasing maps.

Result:

Alloc counter test suitable for any fix of #1697.

@Lukasa Lukasa added the needs-no-version-bump For PRs that when merged do not need a bump in version number. label May 25, 2021
Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good once limits are added.

//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2017-2019 Apple Inc. and the SwiftNIO project authors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Copyright (c) 2017-2019 Apple Inc. and the SwiftNIO project authors
// Copyright (c) 2021 Apple Inc. and the SwiftNIO project authors

Motivation:

Peter thinks that result-erasing maps should not allocate, and we have
special code paths in the code to try to make Void -> Void maps not
allocate. Sadly, both code paths currently do allocate.

Per our rules for not trying to make optimizations without data, we
should start measuing these closures so we can make optimizations.

Modifications:

- Added an alloc couter test for result-erasing maps.

Result:

Alloc counter test suitable for any fix of apple#1697.
@Lukasa Lukasa force-pushed the cb-alloc-test-for-convert-to-void branch from 67ef93b to 7c076ad Compare May 25, 2021 09:18
@Lukasa Lukasa merged commit 43fa768 into apple:main May 25, 2021
@Lukasa Lukasa deleted the cb-alloc-test-for-convert-to-void branch May 25, 2021 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-no-version-bump For PRs that when merged do not need a bump in version number.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants