Skip to content

[WIP] Swift experiments #8670

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

Open
wants to merge 59 commits into
base: main
Choose a base branch
from

Conversation

mortenbekditlevsen
Copy link
Contributor

@mortenbekditlevsen mortenbekditlevsen commented Sep 20, 2021

I’m opening this PR to initiate a discussion about what it would take to replace parts of the suite of modules with swift - and in the long run make some of the modules cross-platform.

I started with looking at FirebaseDatabase, since it is a framework that I have been using and loving for many years now.

My first step was to create a swift framework (FirebaseDarabaseSwiftCore) that can hold the converted code.

Second step is to move types without dependencies to this framework.

I started out with SocketRocket and APLevelDB since they both have no dependencies on other Firebase code and also because they might be challenging.

I replaced socketrocket with swift-nio due to my cross-platform wish, but in order for this to be usable on Darwin platforms there should also be support for URLSession.websocketTask (like the current watchOS solution) as well as a fallback to something else - which could be StarScream or even an objective-c dependency containing the current socketrocket implementation. It is fairly easy to inject the desired dependencies- the interesting thing for me here is whether it’s possible to have an SPM requirement which is only checked out on certain platforms… but I guess you can.

I’ll continue this wall of text later. :-)

@google-cla google-cla bot added the cla: yes label Sep 20, 2021
@mortenbekditlevsen
Copy link
Contributor Author

Please also note that this is a spike or an exploration, so not at all production ready code. 😊

Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

Great to see the Swiftification! Thanks for sharing @mortenbekditlevsen

We'll provide some more detailed feedback later this week.

@mortenbekditlevsen
Copy link
Contributor Author

Note that I had to fork leveldb in order to only include non-c++ headers in the publicHeaders path. This may give issues with the way leveldb is used from Firestore. (I haven't tried it yet).

@mortenbekditlevsen
Copy link
Contributor Author

I'm looking into replacing FImmutableSortedDictionary with a swift version. I incorrectly remembered that swift-collections included a SortedDictionary, but that's an OrderedDictionary, which is not the same. There is a PR for a SortedDictionary and it is actively being discussed on the swift forums.
So for now I am 'stubbing' the use with OrderedDictionary - performing a sort by key after each insertion - knowing full well that it doesn't meet the expected insertion times. I expect a swift-collections 1.1 to include SortedDictionary, and that work will likely be in place looong before these current explorations are over. :-)

@mortenbekditlevsen
Copy link
Contributor Author

Here are my vague thoughts about how a possible transition to swift could be achieved; and this is what I'm exploring with FirebaseDatabase:

Step 1:
Convert code one to one from obj-c to swift while trying to keep the objc-c api the same or very similar.

Step 2:
Remove obj-c annotations on internal api, possibly rewrite tests (huge one). Start testing on Linux.

Step 3:
Convert to 'good' swift: make appropriate types value-types. Perhaps implement cow where applicable. Make collections generic, convert to swift naming conventions, etc. etc.

@paulb777
Copy link
Member

Thanks @mortenbekditlevsen. It's unlikely that we'll be interested in officially supporting Firebase for Linux anytime soon. We're much more interested in exploring Swift API improvements for Apple platform developers and at most would consider Linux as a community supported feature.

Linux would also need to consider a strategy for Firebase Core and Firebase Auth.

Also since this is potentially a large project, before merging, we would need thorough design docs including migration analysis, along with a robust test strategy.

@mortenbekditlevsen
Copy link
Contributor Author

Thanks @mortenbekditlevsen. It's unlikely that we'll be interested in officially supporting Firebase for Linux anytime soon. We're much more interested in exploring Swift API improvements for Apple platform developers and at most would consider Linux as a community supported feature.

Hi @paulb777 ,
I can certainly understand why cross platform support is not on the top of your roadmap. :-)

As such, I can imagine, that at the very least you would only be interested in a subset of the changes I am exploring. For instance I guess that you would have no use for a swift-nio implementation of the websocket communication.

For my own enjoyment, I think that adding Linux (and other) cross platform support is a fun project - and in the fulness of time, I would also hope that this work could be used to implement firebase functions support for a swift runtime.

I could also see a maintenance benefit in moving to Swift - and perhaps even a performance benefit by replacing a lot of dynamic dispatching with static.

But I am of course also aware of the up front cost of making such a switch.

Only having a bit of spare time to experiment, it takes me quite a long time just to convert a few hundred lines of Obj-C to Swift - and there's about 14.000 lines still to go (not even counting comments, header files or any tests).

Linux would also need to consider a strategy for Firebase Core and Firebase Auth.

Certainly. My first goal is to see if I can make a FirebaseDatabase client build by stubbing out those dependencies - and initially only accessing a completely open database.

Also since this is potentially a large project, before merging, we would need thorough design docs including migration analysis, along with a robust test strategy.

Right. And for now I still consider this an early exploration. Once I have something building, I could perhaps help out with design docs and migration analysis.

Let me know if you wish me to close the PR for now - I opened it after chatting with @peterfriese and thought it would be a good way to get a discussion going. :-)

@paulb777
Copy link
Member

@mortenbekditlevsen I wanted to make sure that we're aligned on expectations and it sounds like we generally are, so I'm fine to keep the PR going.

If others in the community want to participate, it might make sense to set up a long-lived branch for incremental PRs, since the PR to master may become too large to understand all at once.

@paulb777
Copy link
Member

I know this is different from your proposed plan, but it would help us to collaborate more effectively if we could separate the Linux changes from the the Swift API improvement changes.

At a high level, while we will likely eventually migrate our SDKs to Swift implementations, the initial priority is to focus on the APIs without necessarily changing the full implementations.

@mortenbekditlevsen
Copy link
Contributor Author

Hi @paulb777 ,
Just to clarify: With Swift API improvements, do you mean end user facing API changes?
Because I'm not really planning to do any of those in this experiment.
I'm (just?) replacing the current Obj-C implementation with a pure Swift one.
I'm sure it will allow for some API changes, but those could also be done 'on top' of the current FirebaseDatabase framework, like is done in FIrebaseDatabaseSwift.

Or do you mean 'general improvements to the codebase' due to the fact that it will be in Swift and we can use value types, generics and all the other goodness that Swift brings to the table?
If so, then I believe that there are plenty of those to be made, but I am not ready to doing them yet, since I am trying to keep tests running, and thus pretty much doing a one-to-one translation.

Together with a pure Swift implementation of FirebaseDatabase, there are some questions regarding the third party code.
For one, SocketRocket is in Objective-C. What should the Swift equivalent be? StarScream (which to my knowledge is close to what socketrocket does)? Or URLSession's websocketTask where available? Or would swift-nio even be feasible for darwin where websocketTask is not available?
Those are questions I can't answer by myself, so for now I focused on swift-nio.
Similarly for the FImmutableSortedDictionary I suggest a replacement from swift-collections that will hopefully land in a future 1.1, namely SortedDictionary. Would you be ok with adding more dependencies? Or perhaps just copying the used parts of the implementation into a third_party folder?
As a replacement for APLevelDB I have used https://github.com/emilwojtaszek/leveldb-swift as a base, but it is a bit outdated. My current suggestion emulated the API from APLevelDB (to make it easier to make the transition), but the final version wouldn't of course need to emulate the API one-to-one.

The reason that I was starting with these dependencies is that for one they don't have any code dependencies of their own, so they're a natural place to start when doing the conversion from 'the bottom'. Secondly they were a bit of a 'dark horse', so I wanted to see if I could tackle them at all.

So for me the next steps are converting stuff from the bottom: FPath, FNode, FIndex, FRepoInfo, FUtilities, FSnapshotUtilities, etc, etc, etc. working my way up from the bottom and trying to keep tests running all along.

So all of this was just to make certain that I explained what I'm doing. To answer your question about leaving the Linux-part aside for now, then that 'just' means providing another websocket implementation in Swift (even though Darwin can fully well run with the swift-nio implementation). For that it would be good to start with the discussion about what to replace it with.

And still I consider all of my current efforts to be an exploration and not an actual proposal for inclusion in the official firebase sdk (for now). :-)

@paulb777
Copy link
Member

Hi @mortenbekditlevsen

Yep - I mean that the sooner priority for the Firebase team will be end user facing API changes along with any implementation conversions to Swift necessary to support those improvements.

I did not mean to suggest putting aside the Linux-part - I just meant that the Firebase team may not have a lot of bandwidth to help with it in the near-term. However, we're quite interested in what your experiments learn.

@mortenbekditlevsen
Copy link
Contributor Author

Hi @paulb777 and others,
I now have the real time database support working on Linux! No auth / app check yet and I stubbed out the FIRApp from Firebase Core.
There are tons of todos, but I think it's a really good start.
The persistence part is supported too.
I relied heavily on the unit tests written in Objective-C during the porting, but for the final step of making this work on Linux I removed all objc annotations and NSObject subclassing. I naively thought that it would just be ignored on non-Darwin platforms, but it turns out that it doesn't build at all with these annotations, so any objc-support would basically have to be re-added as an overlay hidden behind platform checks.
Due to relying so heavily on the tests I have for a large part attempted to port classes one to one. But I have replaced quite a few internals with Swift-specific code including generics and enums with associated types.

There's many todos and still lacking proper logging support (which I'll implement using swift-log, so that different logging mechanisms can be added using dependency injection.

One thing that I am missing is a clear goal. :-)

By that I mean: I have my own clear goal, but IF any of this would ever be embraced by the official project, these goals would look quite different.

Here are a few directions this could be taken:

  1. Disregard Objective-C support since this can be achieved by the official project.
  2. Embrace swift concurrency since this would clean up the code base a lot, but would limit usability on Darwin platforms somewhat.
  3. Keep Objective-C support and don't embrace swift concurrency.

Even choosing 3, if one were to imagine a pure Swift (and C and C++) implementation of the entire project, there's still the whole component registration which relies on objc +load. This of course wont work in pure Swift, so perhaps some sort of manual registration / dependency injection would be needed.

So since that conceptually breaks part of the current 'ease of use' of the entire firebase project I am tempted to say that perhaps 3. isn't the best way to go. And if it isn't, I'd love to continue on with 1. and 2. :-)

The 15.000 lines of unit tests ought to be ported too, since after stripping objective-c support I basically don't have coverage anymore. Note that this branch still has the annotations and all tests pass, but I have another branch with the annotations stripped, and that is the branch that I am using from Linux.

With regards to testing the bottom-most approx. 18.000 lines of code have really good coverage, but the 'top layers' of the actual public API, FPersistentConnection, FRepo and FRepoManager are not tested very thoroughly, so that part of the port is likely less 'reliable' so far, since I don't have tests to support the conversion.

For my own personal exploration, I think that I would like to attempt porting part of Auth as well to get that key component working. But before that I have to figure out the role of GTMSessionFetcher and see if that is something that can either be replaced or if it's something that needs porting too.

Please let me know if you think that this is interesting in any way, or if I should just continue all of this outside of the scope of the official project. :-)

@paulb777
Copy link
Member

Great progress @mortenbekditlevsen! Thanks for sharing.

Some somewhat inconsistent points to think about for merging back:

  • While we'd like to start migrating our implementations to Swift, we expect to need to continue to support Objective-C legacy code for several years
  • With FirebaseFunctions and FirebaseStorage, we found @objc to be a viable way to continue legacy Objective-C API coverage while also adding Swift-only APIs for things like default arguments and async API support
  • It should be fine to port unit tests to Swift. We were able to do a 1-1 port for FirebaseFunctions.
  • The RTDB integrations tests should continue to run as well.
  • Moving internals to rely on Swift concurrency might be ok for our next major release, but we'll need to do more assessment around the trade-offs of requiring an iOS 13 minimum.
  • If this cost of keeping Objective-C support gets too expensive or too restrictive, we can reassess the need to keep in future major releases and consider some kind of ongoing support model for an old major release stream
  • For resolving the +load, component registration issue, we added a registerSwiftComponents function. See https://github.com/firebase/firebase-ios-sdk/blob/master/FirebaseCore/Sources/FIRApp.m#L837
  • In porting FirebaseFunctions and FirebaseStorage, we found that comprehensive Swift and Objective-C API tests that at least verify build compatibility to be very valuable.
  • cc: @thomasvl for any thoughts about GTMSessionFetcher

@thomasvl
Copy link
Contributor

Why does GTMSessionFetcher have to be changed at all? i.e. - just because Firebase were to move to more Swift, that doesn't seem like it means it can't depend on some thing that weren't Swift, no? For that matter, seems like individual things could be migrated one at a time, no?

I guess the biggest issue for migrating in general would be the headers are currently documented as importable into ObjC code, and if implementation moved to Swift, some of those headers would likely go away be replaced by generated headers or you'd have to hand maintain them while supporting both.

@paulb777
Copy link
Member

To clarify, while Firebase is migrating to more Swift, this external PR is exploring the broader question about porting Firebase Database to Swift and to running on Linux.

I suspect the bigger question than the Objective-C to Swift one, is the implications of porting/using GTMSessionFetcher on Linux. And before that we might want to determine what exactly is FirebaseAuth using GTMSessionFetcher for.

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

Successfully merging this pull request may close these issues.

3 participants