[Breaking Changes] Introduce InoutRef object - Used in commit() to detect changes are occurred. #180

muukii · 2020-10-24T19:39:26Z

Main purpose: Store wants to skip a mutation had no changes to reduce emitting meaningless events.

https://github.com/apple/swift-evolution/blob/master/proposals/0205-withUnsafePointer-for-lets.md

Currently: if we do like following, the store emits updates to all of the subscribers.

commit { _ in }

commit {
  if false {
    $0.xxx = something
  }

This PR make Store skips emitting by these commits.

How we detect changes that have occurred is by modifying the properties through KeyPath (dynamicMemberLookup)
However, Wrapping the state causes copying in Swift. So we can avoid that by using UnsafeMutablePointer<T> on UnsafeInoutReference<Wrapped>.

Breaking changes

Using `replace` method

Inside of commit, we become to can not replace with new value by assignment operator.
Instead, we use .replace

Before

commit {
  $0 = value
}

After

commit {
  $0.replace(with: value)
}

`StateType` functionalities will be no longer meaningful.

StateType provides some stuff to modify the value easily.
And the reason why we created this, we had no way to wrap the state to provide helper methods.
But now, we have wrapper object UnsafeInoutReference, this will have methods such as StateType potentially in the future.

muukii · 2020-10-24T19:43:19Z

Sources/VergeCore/Inout.swift

+  private let pointer: UnsafeMutablePointer<State>
+
+  public init(pointer: UnsafeMutablePointer<State>, onModified: @escaping () -> Void) {


Passing the concrete instance causes a copy operation.
To escape from this, use UnsafeMutablePointer.
I want to get behavior programmatically such as well as inout attribute in Swift language.

All of the methods to access the properties of the State use this pointer.

I don't know this works well and correctly.

In addition, different from inout attribute, developers can do escaping Inout object to anywhere.
this is the most dangerous scenario.

(inout Inout<Wrapped>) -> probably works well? to prevent escaping in compile-time.

If it work, I'll change the name of Inout<Wrapped> because it would be confusing.

Changed UnsafeInoutReference<Wrapped>

muukii · 2020-10-25T08:51:24Z

It didn't work well in production

muukii · 2020-10-25T09:38:45Z

It didn't work well in production

I misunderstood the usage of withUnsafeMutablePointer.
I changed to performing all inside withUnsafeMutablePointer.

muukii · 2020-10-25T11:35:18Z

Sources/VergeCore/Storage.swift

+  }
+
+  @inline(__always)
+  public final func _update(_ update: (inout Value) throws -> UpdateResult) rethrows {


We can choose if updates have occurred with UpdateResult

muukii · 2020-10-25T11:35:40Z

Sources/VergeCore/UnsafeInoutReference.swift

+import Foundation
+
+@dynamicMemberLookup
+public final class UnsafeInoutReference<Wrapped> {


This is new object

muukii · 2020-10-25T11:37:26Z

Sources/VergeStore/BatchCommit.swift

+/**
+ A context to batch multiple mutations
+ */
+public struct BatchCommitContext<State, Scope> {


Just moved from Store.swift

muukii · 2020-10-25T11:38:07Z

Sources/VergeStore/Store/Derived.swift

@@ -130,7 +130,7 @@ public class Derived<Value>: _VergeObservableObjectBase, DerivedType {
        break
      case .updated(let newState):
        store?.commit {
-          $0 = newState
+          $0.replace(with: newState)


The reason why we use this method to replace value is we can't override = operator.

muukii · 2020-10-25T11:39:26Z

Sources/VergeStore/Store/DispatcherType.swift

@@ -77,7 +77,7 @@ extension DispatcherType {
    _ file: StaticString = #file,
    _ function: StaticString = #function,
    _ line: UInt = #line,
-    mutation: (inout Scope) throws -> Result
+    mutation: (inout UnsafeInoutReference<Scope>) throws -> Result


The reason why we still use inout attribute is to restrict escaping the reference object by developers in compile time.

muukii · 2020-10-25T12:04:43Z

Tests/VergeStoreTests/VergeStoreTests.swift

+  func testEmptyCommit() {
+
+    let store = DemoStore()
+
+    var count = 0
+
+    let subs = store.sinkState(queue: .passthrough) { (_) in
+      count += 1
+    }
+
+    XCTAssertEqual(store.state.version, 0)
+
+    store.commit {
+      $0.count = 100
+    }
+
+    XCTAssertEqual(store.state.version, 1)
+
+    store.commit { _ in
+
+    }
+
+    // no changes
+    XCTAssertEqual(store.state.version, 1)
+
+    store.commit {
+      // explict marking
+      $0.markAsModified()
+    }
+
+    // many times calling empty commits
+    for _ in 0..<3 {
+      store.commit { _ in }
+    }
+
+    // no affects from read a value
+    store.commit {
+      if $0.count > 100 {
+        $0.count = 0
+        XCTFail()
+      }
+    }
+
+    XCTAssertEqual(store.state.version, 2)
+    XCTAssertEqual(count, 3)
+
+    withExtendedLifetime(subs, {})

+  }


Test code is here.
we can see the version is not updated after empty-commit.

muukii · 2020-10-25T12:16:19Z

Now, trying on my application.

muukii commented Oct 24, 2020

View reviewed changes

muukii added Group: Enhancement Tag: New Feature Tag: Performance labels Oct 24, 2020

muukii changed the title ~~[WIP / Experimental] Introduce Inout object - Used in commit() to detect changes are occurred.~~ Introduce UnsafeInoutReference object - Used in commit() to detect changes are occurred. Oct 25, 2020

muukii added 16 commits October 25, 2020 20:21

Add Inout

7013826

Add tests

518128b

Rename

b4d638b

⚡ WIP

ef687ce

🌲 Update

fd248ef

perform inside withUnsafe

9937612

Update tests

5ddbf7b

Fix tests

3f88b12

Remove ReadRef

d1749d6

Fix

b773c43

Add inout attributes

9ac50eb

Move BatchCommit

856afb3

Fix tests

b379871

Update Xcode version

8229105

Fix tests

2b756d6

Use _read _modify

dcee035

muukii force-pushed the muukii/inout branch from 69977e8 to dcee035 Compare October 25, 2020 11:21

Fix tests

5e79a0e

muukii changed the title ~~Introduce UnsafeInoutReference object - Used in commit() to detect changes are occurred.~~ [Breaking Changes] Introduce UnsafeInoutReference object - Used in commit() to detect changes are occurred. Oct 25, 2020

muukii added the Tag: Breaking Changes label Oct 25, 2020

muukii self-assigned this Oct 25, 2020

🌲 Update

caabc45

muukii commented Oct 25, 2020

View reviewed changes

Update UnsafeInoutReference

8858191

muukii commented Oct 25, 2020

View reviewed changes

muukii marked this pull request as ready for review October 25, 2020 12:11

muukii requested review from shima11 and takumatt October 25, 2020 12:11

takumatt approved these changes Oct 26, 2020

View reviewed changes

Rename

9690759

muukii changed the title ~~[Breaking Changes] Introduce UnsafeInoutReference object - Used in commit() to detect changes are occurred.~~ [Breaking Changes] Introduce InoutRef object - Used in commit() to detect changes are occurred. Oct 27, 2020

muukii merged commit 7b48f36 into master Oct 29, 2020

muukii deleted the muukii/inout branch October 29, 2020 21:13

muukii restored the muukii/inout branch October 29, 2020 21:13

muukii deleted the muukii/inout branch December 29, 2020 04:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Breaking Changes] Introduce InoutRef object - Used in commit() to detect changes are occurred. #180

[Breaking Changes] Introduce InoutRef object - Used in commit() to detect changes are occurred. #180

muukii commented Oct 24, 2020 •

edited

muukii Oct 24, 2020

muukii Oct 24, 2020

muukii Oct 24, 2020 •

edited

muukii Oct 24, 2020

muukii Oct 25, 2020

muukii commented Oct 25, 2020

muukii commented Oct 25, 2020

muukii Oct 25, 2020

muukii Oct 25, 2020

muukii Oct 25, 2020

muukii Oct 25, 2020

muukii Oct 25, 2020

muukii Oct 25, 2020

muukii commented Oct 25, 2020

		private let pointer: UnsafeMutablePointer<State>

		public init(pointer: UnsafeMutablePointer<State>, onModified: @escaping () -> Void) {

[Breaking Changes] Introduce InoutRef object - Used in commit() to detect changes are occurred. #180

[Breaking Changes] Introduce InoutRef object - Used in commit() to detect changes are occurred. #180

Conversation

muukii commented Oct 24, 2020 • edited

Breaking changes

Using replace method

StateType functionalities will be no longer meaningful.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muukii Oct 24, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muukii commented Oct 25, 2020

muukii commented Oct 25, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

muukii commented Oct 25, 2020

muukii commented Oct 24, 2020 •

edited

Using `replace` method

`StateType` functionalities will be no longer meaningful.

muukii Oct 24, 2020 •

edited