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

Build a MVP of macro-based model classes #8270

Closed
sync-by-unito bot opened this issue Jun 12, 2023 · 9 comments
Closed

Build a MVP of macro-based model classes #8270

sync-by-unito bot opened this issue Jun 12, 2023 · 9 comments
Assignees

Comments

@sync-by-unito
Copy link

sync-by-unito bot commented Jun 12, 2023

Build something with the minimal set of features which could plausibly be actually used. The primary goal here is to get a better understanding of what is and isn't possible and how difficult various things are to enable scoping out the full feature.

@philipbel
Copy link

It would be great to get rid of the Mirror-based reflection and allow for non-optional properties (see #1615).

@aehlke
Copy link

aehlke commented Oct 11, 2023

post that as a new issue

@rjchatfield
Copy link
Contributor

👋 Hi there. I'm curious how this investigation went. I see this is closed and there is a slightly older Discussion about Initialisation/Migration Performance #8194 (reply in thread). But no details that I could find.

We have a reasonably large Realm schema that takes a long time dynamically infer at app launch (~700ms), and the idea of moving that from customer's run time back to dev compile time is very apealling.
Specifically, it would be great to avoid:

  • Avoiding a call to objc_copyClassList
  • Avoiding a call to schemaForObjectClass for all Objects

Thank you. :)

@tgoyne
Copy link
Member

tgoyne commented Dec 14, 2023

The conclusion of the prototype is that it's something which can work and wouldn't be super difficult to implement, but there's significant problems with depending on SwiftSyntax (it takes longer to compile than Realm does and people are understandable already unhappy with our build times, and it's not versioned in a SPM-compatible way).

@rjchatfield
Copy link
Contributor

@tgoyne Thanks for replying. I really appreciate it. :)

which can work and wouldn't be super difficult to implement

Oh that's very pleasing news.

there's significant problems with depending on SwiftSyntax

Yeah this is a bit rough. We're all glad that you didn't merge in something like that which would effect all of your customers.

Is there any other option? As I mentioned, my team (and our customers) would very much appreciate something like this to help significantly shrink boot times. Is this something you could maybe implement as a stand alone realm/realm-swift-macros library that consumers could opt-in to? Or perhaps less officially, would you mind sharing what you'd done and allow us to implement it ourselves?

@tgoyne
Copy link
Member

tgoyne commented Dec 16, 2023

I think #8261 has everything I wrote before moving onto other things. It is distinctly not very tested beyond incidentally testing it by switching all the object types in our other tests to use it, but IIRC those mostly worked. It's not a purely additive thing and requires a hook for schema discovery, but we could maybe expose that hook with the obvious caveat of that it would be prone to breaking when we change internals.

@rjchatfield
Copy link
Contributor

Thanks for sharing that. All very cool.

The part I find most valuable is the @RealmDiscoverMacro. The synthesising of the RLMProperty's is the slowest part of Schema generation (second slowest is the detection of Realm Objects... but let's handle that separately). The other Macro aims to make Realm Objects feel more like Swift's @Model type, which is nifty, but I'll leave that up to your team to prioritise... but since it doesn't speed up the runtime problem it's not a priority to my project.

It seems fairly simple for us to implement this Swift Macro ourselves, except of the changes in RealmSwift:

  • _RealmObjectSchemaDisoverable protocol and the detection of it inside the getProperties.
  • new initialisers of RLMProperty.
    Did I miss something?

If we wanted to have these extension points added to RealmSwift, is there an evolution process for "pitching" these kinds of changes? Do I make separate ticket on GitHub or something?

Thanks again for sharing.

@tgoyne
Copy link
Member

tgoyne commented Dec 19, 2023

If you put together something that just needs a minor hook then I don't think including that would be a problem. Note that the RLMProperty changes don't need to be in realm-swift; they're just convenience initializers that could be put anywhere or even just functions.

@rjchatfield
Copy link
Contributor

:goodnews:

I've got an update for anyone following this discussion. I've pushed up my attempt at a Swift Macro for generating RLMProperties for an annotated Object.

It halved the time taken for RLMSchema.shared().

Open to feedback. 😁

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants