Skip to content

Lock: changed default implementation to use NSLock#1819

Merged
NachoSoto merged 5 commits into
mainfrom
lock-recursive
Aug 22, 2022
Merged

Lock: changed default implementation to use NSLock#1819
NachoSoto merged 5 commits into
mainfrom
lock-recursive

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

Atomic can't be used recursively (due to Swift's inout limitation), so we pay an unnecessary price in performance using NSRecursiveLock that we don't need.
See https://swiftrocks.com/thread-safety-in-swift for a benchmark.

The implementation still allows using NSRecursiveLock when necessary.

@NachoSoto NachoSoto added the perf label Aug 13, 2022
@NachoSoto NachoSoto requested a review from a team August 13, 2022 16:12
@NachoSoto

Copy link
Copy Markdown
Contributor Author

Bump @RevenueCat/core-sdk

Comment thread Sources/Misc/Lock.swift Outdated
private init(_ lock: UnderlyingType) { self.lock = lock }

/// Initializes an instance backed by a non-recursive `NSLock`
convenience init() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

kinda wonder if it now makes sense to have something like createNonRecursive so it's very clear when you're using them and you don't miss the fact that there's a choice.
or even lock = Lock(.recursive) and lock = Lock(.nonRecursive)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah totally 👍🏻 I'll change this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done!

`Atomic` can't be used recursively (due to Swift's `inout` limitation), so we pay an unnecessary price in performance using `NSRecursiveLock` that we don't need.
See https://swiftrocks.com/thread-safety-in-swift for a benchmark.

The implementation still allows using `NSRecursiveLock` when necessary.
@NachoSoto NachoSoto merged commit ba3e61c into main Aug 22, 2022
@NachoSoto NachoSoto deleted the lock-recursive branch August 22, 2022 23:44
@vegaro vegaro mentioned this pull request Aug 26, 2022
kaushikpaperboat pushed a commit to Kiddopia/purchases-ios that referenced this pull request Sep 7, 2022
* rc-4.11.0: (964 commits)
  Update RevenueCat-Swift.h for version 4.11.0
  Version bump for 4.11.0
  skip if needed and adds automatic_release (RevenueCat#1870)
  Fixed crash on `async` SK1 cancelled purchase (RevenueCat#1869)
  Added `beginRefundRequest` overload with completion block (RevenueCat#1861)
  Release/4.10.3 (RevenueCat#1867)
  Update fastlane-plugin-revenuecat_internal and fix release-train job (RevenueCat#1866)
  fix typo in comment (RevenueCat#1863)
  Use Dangerfile repository (RevenueCat#1864)
  `CircleCI`: added job for building SDK with `SPM` (RevenueCat#1860)
  `TrialOrIntroPriceEligibilityChecker`: return `.noIntroOfferExists` if the product has no introductory offer (RevenueCat#1859)
  `Lock`: changed default implementation to use `NSLock` (RevenueCat#1819)
  `Offering: Sendable` conformance (RevenueCat#1826)
  `ReceiptParser: Sendable` conformance (RevenueCat#1825)
  `CustomerInfo: Sendable` conformance (RevenueCat#1824)
  Added `Collection.onlyElement` (RevenueCat#1857)
  README updates (RevenueCat#1856)
  `IntegrationTests`: actually fail test if tests aren't configured (RevenueCat#1855)
  `watchOS`: fixed crash on single-target apps (RevenueCat#1849)
  Prepare next version: 4.11.0-SNAPSHOT (RevenueCat#1854)
  ...

# Conflicts:
#	.circleci/config.yml
#	.jazzy.yaml
#	.version
#	CHANGELOG.latest.md
#	CHANGELOG.md
#	Gemfile.lock
#	Package.swift
#	Purchases/Attribution/RCAttributionFetcher.h
#	Purchases/Attribution/RCAttributionFetcher.m
#	Purchases/Attribution/RCAttributionPoster.m
#	Purchases/Attribution/RCAttributionTypeFactory.h
#	Purchases/Attribution/RCAttributionTypeFactory.m
#	Purchases/Caching/RCDeviceCache.m
#	Purchases/Misc/RCSystemInfo.m
#	Purchases/Networking/RCBackend.h
#	Purchases/Networking/RCBackend.m
#	Purchases/ProtectedExtensions/RCAttributionTypeFactory+Protected.h
#	Purchases/Public/RCPurchases.h
#	Purchases/Public/RCPurchases.m
#	Purchases/Public/RCPurchasesErrorUtils.h
#	Purchases/Public/RCPurchasesErrorUtils.m
#	Purchases/Purchasing/RCOfferingsFactory.m
#	Purchases/Purchasing/RCStoreKitRequestFetcher.h
#	Purchases/Purchasing/RCStoreKitRequestFetcher.m
#	Purchases/SubscriberAttributes/RCSubscriberAttributesManager.h
#	Purchases/SubscriberAttributes/RCSubscriberAttributesManager.m
#	PurchasesCoreSwift.podspec
#	PurchasesCoreSwift/Info.plist
#	PurchasesCoreSwift/Logging/Strings/AttributionStrings.swift
#	PurchasesCoreSwift/Logging/Strings/IdentityStrings.swift
#	PurchasesCoreSwift/Logging/Strings/OfferingStrings.swift
#	PurchasesCoreSwift/Misc/OperationDispatcher.swift
#	PurchasesCoreSwift/SubscriberAttributes/SpecialSubscriberAttributes.swift
#	PurchasesTests/Info.plist
#	PurchasesTests/Mocks/MockAttributionFetcher.swift
#	PurchasesTests/Mocks/MockAttributionTypeFactory.swift
#	PurchasesTests/Mocks/MockBackend.swift
#	PurchasesTests/Mocks/MockOfferingsFactory.swift
#	PurchasesTests/Mocks/MockRequestFetcher.swift
#	PurchasesTests/Networking/BackendTests.swift
#	PurchasesTests/Purchasing/OfferingsTests.swift
#	PurchasesTests/Purchasing/PurchasesTests.swift
#	PurchasesTests/SubscriberAttributes/PurchasesSubscriberAttributesTests.swift
#	RevenueCat.podspec
#	Sources/Info.plist
#	Tests/BackendIntegrationTestApp/Info.plist
#	Tests/BackendIntegrationTests/Info.plist
#	Tests/InstallationTests/CocoapodsInstallation/Gemfile.lock
#	Tests/InstallationTests/XcodeDirectInstallation/XcodeDirectInstallation.xcodeproj/project.pbxproj
#	Tests/UnitTests/Attribution/AttributionTypeFactoryTests.swift
#	Tests/UnitTests/Info.plist
#	Tests/UnitTests/Mocks/MockProductsRequest.swift
#	Tests/UnitTests/Mocks/MockSubscriberAttributesManager.swift
#	Tests/UnitTests/Purchasing/StoreKitRequestFetcherTests.swift
#	fastlane/Fastfile
@vegaro vegaro added pr:other and removed pr:perf labels Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants