search

ancapdev / LightBSON.jl

High performance encoding and decoding of BSON data in Julia
MIT License
20 stars 4 forks source link

NamedTuple dispatch #24

Closed poncito closed 1 year ago

poncito commented 1 year ago

Hi Christian,

I'm not quite sure about this method. I would just remove it in order to be able to benefit from the bson_write_simple optimization. Would you agree?

Then, I'm wondering why you're using this default,

bson_simple(::Type{T}) where T = StructTypes.StructType(T) == StructTypes.NoStructType()

In the case of the namedtuples, they are StructTypes.UnorderedStruct, and hence are not treated as simple bsons by default. My expectation was that a namedtuple carries no information on its type, and hence I'd be surprise if we would ever consider them as more than just a collection of fields to serialize.

What am I missing? Thanks, Romain

ancapdev commented 1 year ago

I think you're right, might have been the order in which things were implemented or by omission. This could probably be done better by removing that method and overriding bson_simple for named tuples. Is there something you need it for now, or just an observation?

poncito commented 1 year ago

Hi, Cool. I'll propose a small PR then. That will give a nice improvement in peformance with namedtuples! I'm preparing a bigger PR related to schema management, providing a few features that I would really enjoy using. So I'm reading the module's code and this dispatch caught my eyes. I don't really "need it", just a simple improvement before discussing about the bigger PR. Thanks,

ancapdev commented 1 year ago

Fixed in #25