Skip to content

Conversation

@filosganga
Copy link
Collaborator

@filosganga filosganga commented Dec 30, 2024

This PR fundamentally solve the FreeApplicative at schema creation and replaces it with a custom structure that is flat, avoiding to navigate the FreeApplicative structure each time we encode or decode a Record.

@filosganga filosganga requested a review from SystemFw December 30, 2024 20:06
@filosganga filosganga changed the title Caching Improve performance of encode and decode an M Dec 30, 2024
Comment on lines +174 to +183
case class OptimizedRecord[R, A](
fields: List[Field[R, _]],
build: List[Any] => A
)

def fields[R](p: FreeApplicative[Field[R, *], R]): Schema[R] = {
val optimized = optimizeRecord(p)
Record(optimized.fields, optimized.build)
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@SystemFw this is the core of the thing

Comment on lines +296 to +310
case class Record[R](
fields: List[Field[R, ?]],
build: List[Any] => R,
fieldNames: Array[String]
) extends Schema[R]

object Record {
def apply[R](
fields: List[Field[R, ?]],
build: List[Any] => R
): Record[R] = {
val fieldNames = fields.map(_.name).toArray
new Record(fields, build, fieldNames)
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure about this, I think we could replace with:

case class Record[R](or: OptimizedRecord[R,*]) extends Schema[R]

I don't believe caching fieldNames make this huge difference in the normal use cases

@filosganga
Copy link
Collaborator Author

I have moved internal/encoding and internal/decoding into the JS/JVM specific code, as the encodeRecord handles the AttributeValue directly and uses the JVM IdentityMap.

@SystemFw
Copy link
Owner

SystemFw commented Feb 5, 2025

There's been some discussion offline. General direction is good, we just need to iron out some minor details.

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

Successfully merging this pull request may close these issues.

3 participants