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

Swift version #99

Open
andrewbradnan opened this issue May 5, 2016 · 17 comments
Open

Swift version #99

andrewbradnan opened this issue May 5, 2016 · 17 comments

Comments

@andrewbradnan
Copy link

I have a port of Sodium for Swift pretty close. Do you have a sec to take a look? I don't even know the best way to get you a copy. I'm sure you could save me a few days debugging with a few pointers.

Thanks

15:34:00.701 xctest[46731:41871984] _XCT_testBundleReadyWithProtocolVersion:minimumVersion: reply received
15:34:00.704 xctest[46731:41871979] _IDE_startExecutingTestPlanWithProtocolVersion:16
Test Suite 'All tests' started at 2016-05-05 15:34:00.741
Test Suite 'SodiumTests.xctest' started at 2016-05-05 15:34:00.743
Test Suite 'SodiumTests' started at 2016-05-05 15:34:00.743
Test Case '-[SodiumTests.SodiumTests testApply]' started.
Test Case '-[SodiumTests.SodiumTests testApply]' passed (0.004 seconds).
Test Case '-[SodiumTests.SodiumTests testExample]' started.
Test Case '-[SodiumTests.SodiumTests testExample]' passed (0.000 seconds).
Test Case '-[SodiumTests.SodiumTests testHold]' started.
Test Case '-[SodiumTests.SodiumTests testHold]' passed (0.001 seconds).
Test Case '-[SodiumTests.SodiumTests testHoldIsDelayed]' started.
/Volumes/Development/dev/sodium/swift/src/Sodium/Sodium/Sodium/SodiumTests/SodiumTests.swift:300: error: -[SodiumTests.SodiumTests testHoldIsDelayed] : XCTAssertTrue failed - testHoldIsDelayed() failed ["2 0", "3 0"]
Test Case '-[SodiumTests.SodiumTests testHoldIsDelayed]' failed (0.002 seconds).
Test Case '-[SodiumTests.SodiumTests testLift]' started.
Test Case '-[SodiumTests.SodiumTests testLift]' passed (0.001 seconds).
Test Case '-[SodiumTests.SodiumTests testLiftFromSimultaneous]' started.
Test Case '-[SodiumTests.SodiumTests testLiftFromSimultaneous]' passed (0.001 seconds).
Test Case '-[SodiumTests.SodiumTests testLiftGlitch]' started.
/Volumes/Development/dev/sodium/swift/src/Sodium/Sodium/Sodium/SodiumTests/SodiumTests.swift:175: error: -[SodiumTests.SodiumTests testLiftGlitch] : XCTAssertTrue failed - test() failed ["3 5"]
Test Case '-[SodiumTests.SodiumTests testLiftGlitch]' failed (0.001 seconds).
Test Case '-[SodiumTests.SodiumTests testListen]' started.
Test Case '-[SodiumTests.SodiumTests testListen]' passed (0.001 seconds).
Test Case '-[SodiumTests.SodiumTests testListenOnce]' started.
Test Case '-[SodiumTests.SodiumTests testListenOnce]' passed (0.001 seconds).
Test Case '-[SodiumTests.SodiumTests testMap]' started.
Test Case '-[SodiumTests.SodiumTests testMap]' passed (0.001 seconds).
Test Case '-[SodiumTests.SodiumTests testMapLateListen]' started.
/Volumes/Development/dev/sodium/swift/src/Sodium/Sodium/Sodium/SodiumTests/SodiumTests.swift:145: error: -[SodiumTests.SodiumTests testMapLateListen] : XCTAssertTrue failed - testMapLateListen() failed ["6", "8"]
Test Case '-[SodiumTests.SodiumTests testMapLateListen]' failed (0.002 seconds).

Test Case '-[SodiumTests.SodiumTests testPerformanceExample]' started.
/Volumes/Development/dev/sodium/swift/src/Sodium/Sodium/Sodium/SodiumTests/SodiumTests.swift:33: Test Case '-[SodiumTests.SodiumTests testPerformanceExample]' measured [Time, seconds] average: 0.000, relative standard deviation: 134.973%, values: [0.000004, 0.000001, 0.000000, 0.000000, 0.000000, 0.000000, 0.000000, 0.000000, 0.000000, 0.000000], performanceMetricID:com.apple.XCTPerformanceMetric_WallClockTime, baselineName: "", baselineAverage: , maxPercentRegression: 10.000%, maxPercentRelativeStandardDeviation: 10.000%, maxRegression: 0.100, maxStandardDeviation: 0.100
Test Case '-[SodiumTests.SodiumTests testPerformanceExample]' passed (0.359 seconds).
Test Case '-[SodiumTests.SodiumTests testUpdates]' started.
Test Case '-[SodiumTests.SodiumTests testUpdates]' passed (0.001 seconds).
Test Case '-[SodiumTests.SodiumTests testValue]' started.
Test Case '-[SodiumTests.SodiumTests testValue]' passed (0.001 seconds).
Test Case '-[SodiumTests.SodiumTests testValueThenFilter]' started.
Test Case '-[SodiumTests.SodiumTests testValueThenFilter]' passed (0.001 seconds).
Test Case '-[SodiumTests.SodiumTests testValueThenLateListen]' started.
Test Case '-[SodiumTests.SodiumTests testValueThenLateListen]' passed (0.001 seconds).
Test Case '-[SodiumTests.SodiumTests testValueThenMap]' started.
Test Case '-[SodiumTests.SodiumTests testValueThenMap]' passed (0.001 seconds).
Test Case '-[SodiumTests.SodiumTests testValueThenMerge]' started.
/Volumes/Development/dev/sodium/swift/src/Sodium/Sodium/Sodium/SodiumTests/SodiumTests.swift:87: error: -[SodiumTests.SodiumTests testValueThenMerge] : XCTAssertTrue failed - testValueThenMerge() failed [2, 1, 4]
Test Case '-[SodiumTests.SodiumTests testValueThenMerge]' failed (0.011 seconds).
Test Case '-[SodiumTests.SodiumTests testValueThenOnce]' started.
Test Case '-[SodiumTests.SodiumTests testValueThenOnce]' passed (0.001 seconds).
Test Suite 'SodiumTests' failed at 2016-05-05 15:34:01.193.
Executed 19 tests, with 4 failures (0 unexpected) in 0.390 (0.450) seconds
Test Suite 'SodiumTests.xctest' failed at 2016-05-05 15:34:01.194.
Executed 19 tests, with 4 failures (0 unexpected) in 0.390 (0.451) seconds
Test Suite 'All tests' failed at 2016-05-05 15:34:01.194.
Executed 19 tests, with 4 failures (0 unexpected) in 0.390 (0.453) seconds

@the-real-blackh
Copy link

I've created you a new repo at https:/SodiumFRP/sodium-swift and sent an invitation to give you access to it, so please start off by checking it in.

@andrewbradnan
Copy link
Author

Sorry, I added to origin/swift but I can move it no problem.
Everything (well unit tests that is) working now but one.

One question on the following test. I'm not what the intended difference is between a Cell.listen() and a Stream.listen()

The second assert fails with an empty array. This seems correct to me, but I'm not sure what it's really supposed to do.

    func testHoldStreamListen() {
        let s = StreamSink<Int>()
        let c = s.hold(0)
        XCTAssert(0 == c.sample(), "testHoldStreamListen() sample after hold failed \(c.sample())")
        var out = [Int]()
        do {
            let l = s.listen { out.append($0) }
            defer { l.unlisten() }

            // s.send(2)
            // s.send(9)
        }
        XCTAssert([0] == out, "testHoldStreamListen() failed \(out)")  // out is [] when I run this test
    }

@the-real-blackh
Copy link

the-real-blackh commented May 19, 2016

    public void testHold() {
        StreamSink<Integer> e = new StreamSink<Integer>();
        Cell<Integer> b = e.hold(0);
        List<Integer> out = new ArrayList<Integer>();
        Listener l = Operational.updates(b).listen(x -> {
            out.add(x);
        });
        e.send(2);
        e.send(9);
        l.unlisten();
        assertEquals(Arrays.asList(2, 9), out);
    }

This is really a test of Operational.updates() more than anything, testing whether it gives a stream that works the same as the stream that was held.

The tests are not as good as they could be. I am going to finish my test generator at some point, and this will generate a set of tests that are the same across all languages.

c.listen() is equivalent to Operational.values(c).listen() issued within a transaction. It will call the callback once for the current value. So if you used b.listen() in the above example, the result would be Arrays.asList(0,2,9).
EDIT: Note that without an explicit transaction, the values() and listen() are in separate transactions, so the initial value will not be caught by listen(). This is why Cell.listen() is an important method.
Operational.updates(c).listen() only gives the updates, not the initial value, so that's why the result is Arrays.asList(2,9) for this test.

@andrewbradnan
Copy link
Author

Oh good, then everything is working. Just "porting" the document comments and I'll check into the repo you set up. Thanks.

@the-real-blackh
Copy link

This is beyond awesome! Thank you!

@the-real-blackh
Copy link

A quick Google search tells me that Swift doesn't have a garbage collector...

...which presumably means that we will have problems with cleaning up smart pointer cycles? If so, C++ has the same problem and I'm working on a solution for it.

@andrewbradnan
Copy link
Author

iOS doesn't have a garbage collector, so yes you can have "retain cycles". Just good old fashioned reference counting. There are no weak pointers for closures either, so we may need some box class that we keep weak references to, for NodeTarget.action.

@the-real-blackh
Copy link

In Javascript there are no finalizers or weak references so I am working on tracking all dependencies, which requires occasionally explicitly declaring variables captured in closures. There are also rare cases where a cell holds a Sodium object but this doesn't feed directly into a switch. These will need to be tracked with a function to extract the Sodium objects, but this case may be rare enough that we can just say that it isn't allowed.

I think what we need for both Swift and C++ is firstly, that stuff. That will give us 100% dependency tracking. Once we have that it'll be relatively easy to apply one of those algorithms that cleans up cycles by marking them with different colours.

@the-real-blackh
Copy link

I don't think weak references help much with this problem but I could be wrong.

@andrewbradnan
Copy link
Author

andrewbradnan commented May 21, 2016

Knock on wood, I've been able to clean up any references using weak pointers in the closures. Do you have a Mac to follow along?

    public func map<TResult>(f: (T) -> TResult) -> Stream<TResult>
    {
        let out = Stream<TResult>(keepListenersAlive: self.keepListenersAlive)
        let l = self.listen(out.node, action: { [weak out] (trans2, a) in out!.send(trans2, a: f(a)) } )
        return out.unsafeAddCleanup(l)
    }

I started SodiumCocoa and SodiumExamples to actually use FRP as I read everything again :)

So far I see the correct number of cleanup's. I'm sure there's plenty more to fix...
I'll try and add some more unit tests. Hopefully it won't be too crazy (all the cleanup occurs on background threads to make things interesting).

@the-real-blackh
Copy link

The problem will only occur if you use switch, and the code you're switching contains a CellLoop or StreamLoop. It'll leak memory and waste CPU time. So you can use it for a lot of stuff before you run into problems.

@kaoskorobase
Copy link

Hi Andrew,

I've been referred by Steve to this issue.

Would you mind commenting on the current status of SodiumSwift? Is the issue with retain cycles in code using switch and loop the only (known) issue?

Thanks!

@andrewbradnan
Copy link
Author

I actually haven't even looked at the retain cycles, so I don't know. Haven't even needed switch/loop, so I'm not sure if I've ported them even.

The only test that doesn't work at the moment is ListenOnce, and I just haven't needed it.

I use it. It's great.

feel free and email me at andrew at whirlygigventures dot com since I look at github so often. (sorry about the 3 week late response)

@the-real-blackh
Copy link

the-real-blackh commented Jan 5, 2017

@andrewbradnan The memory management will be wrong now in Swift, like was for Typescript (but fixed now) and still is for C++. I'll be implementing a fix for this in C++ some time soon. The solution for Swift should basically be a direct port of that solution.

@andrewbradnan
Copy link
Author

Cool. Poke me when you are done and I'll port it.

@the-real-blackh
Copy link

the-real-blackh commented Jan 10, 2017

I will! Thanks!

@clinuxrulz
Copy link
Contributor

clinuxrulz commented Nov 17, 2018

There is a working memory management solution now in the Rust version. Rust sits in the same boat as swift, it too does not come with a garbage collector.

https:/SodiumFRP/sodium-rust/blob/master/src/sodium/impl_/gc.rs

I believe swift supports something similar to traits. The solution could be ported to swift.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants