proposal: make a bunch of stuff inlinable

[lukaskollmer]

Current Situation

effectively none of the APIs in FHIRModels can be inlined by apps/packages using the FHIRModels package.
this is can become very painful, especially e.g. when creating a lot of FHIRPrimitives where the underlying value is just a copy of the input (eg: FHIRString, FHIRURI, FHIRDecimal, etc.)

context: our application's primary use case for FHIRModels is creating Observations (and via that a bunch of other FHIR types), which then get JSON-encoded and uploaded to a server. (we also sometimes use the package to decode and inspect data, but that's a tiny minority of our usage.)
it's not uncommon for our app to create millions of FHIR samples per day; as a result the lack of inlinability does actually have a significant and easily noticable impact on our app's performance.

Suggested Solution

i'd like to propose Apple consider marking as much of the FHIRModels package's public API as possible as @inlinable:

• all trivial operations on FHIRPrimitive and its companion types (FHIRString, FHIRURI, FHIRDecimal, FHIRInteger, etc)
  • initializers
  • equatable implementations
  • other operations (eg: stuff like Base64Binary.data(using:), FHIRPrimitiveProtocol.extensions(for:), Instant.asNSDate(), etc)
  • all of the asFHIR{Type}Primitive() extensions
• all initializers on FHIRType-derived classes
  • including the convenience initializers; since they typically default all of their parameters to nil, my understanding is that the compiler would end up emitting code only for setting those properties that actually are passed into the initializer, but i'll need to double-check that)
• all trivial operations on FHIRType and its derived classed (eg: the FHIRAbstractResource hash and equality operations)

there are some things that probably don't make sense to inline (ResourceProxy.get comes to mind; i'm not sure how the compiler would handle that)

[p2-apple]

That's a great suggestion. Have you done some investigation on which @inlineable attributes make a big difference? Some are obvious candidates, as you call out, but for others it would be good to have some data to see if the compiler actually ends up inlining them and what performance increase you get.

The side effect of @inlineable is that the framework will grow in size, and it's already quite large, so we should be very deliberate about this effort.

[lukaskollmer]

@p2-apple i did some very crude benchmarking at the time but nothing overly sophisticated; will take a closer look at this and report back

tbh i'm not overly worried about the size implications; the standard library already defines many of its operations as inlinable and it's totally fine overall (and i'd imagine that to have way more potential impact than the FHIRModels package ever could have). additionally, a function being inlinable only means that the compiler can inline it if it wants to; it doesn't have to do so. eg, if an app is compiled with -Osize, it'd likely skip a bunch of inlining in cases where it'd lead to unacceptable size increases

[p2-apple]

@lukaskollmer I'd be interested to see if the switch from classes to value types in 0.9.0 helps here.

[lukaskollmer]

@p2-apple just saw the release; will try to switch all of our packages over and then let you know if there was a noticable impact.

my instinctive assumption, tbh, would be that from an initialization pov the structs are benefitial, but from a data flow pov it might not necessarily be, since moving a bunch of class instances through a program can be cheaper than moving around structs (you just have the pointer being passed around), whereas the struct instances in the worst case need to be copied every time they are passed into a function/assigned to a variable/etc

i'm not 100% sure about the actual impact here, though; will see if i can set up a sample project that builds up and processes and moves around large amounts of Observations and can easily be switched back and forth between the 0.8.0 and 0.9.0 implementations, so that we can measure this.

in either case, having these types be structs is the way to go IMO (unless the performance impacts are really bad, which i doubt), simply because of the resulting simplicity wrt sendability, mutability and overall correctness (you dont have to worry about some random part of the codebase potentially changing your resources and can treat them as being truly immutable, which will be a great benefit for our codebase — thank you for making this change!)

[p2-apple]

I have the same hunch as you, if you have a way to measure that'd be awesome. Whether there's negative impact due to the switch from classes to structs depends a lot on the use case and how these are used, as you say. LMK what you find – no rush! And thanks for checking.

[PSchmiedmayer]

Thank you @p2-apple; this is huge! 🚀

We will check out the release in detail and start adopting things around Spezi; it will make our life way easier to handle concurrent code going forward. @lukaskollmer and I will collect any feedback that we might have as we look into the code.