search

mbraceproject / FsPickler

A fast multi-format message serializer for .NET
http://mbraceproject.github.io/FsPickler/
MIT License
324 stars 52 forks source link

Why are F# records not cached by ref? #31

Closed Bananas-Are-Yellow closed 9 years ago

Bananas-Are-Yellow commented 9 years ago

Why does FsRecordPickler.Create create a CompositePickler with cacheByRef = false?

An F# record uses structural comparison by default, but if you pass them around, you are passing references. You might store several references to the same record, so why not cache by reference when pickling? It can do no harm, and might do some good.

In my case, I am using [<ReferenceEquality; NoComparison>], because I am using records in a DAG. But even with structural equality, there's no harm in caching by reference, surely?

David.

eiriktsarpalis commented 9 years ago

I did some benchmarks quite some time ago and found that caching "small" objects such as strings, tuples and algebraic types had a degrading effect on performance. Like you said, these are reference types but largely treated as if they were values in F#. They are deliberately not cached (see tuples) unless found to be recursive (i.e. admitting cyclic graph instances). If your record type is recursive then its auto-generated pickler will cache by reference.

That said, it seems to me that this approach is wrong when F# types carry the ReferenceEquality attribute. I will push a fix that addresses this.

Bananas-Are-Yellow commented 9 years ago

I see. Any idea when this fix will be made?

Does this mean the format version gets incremented?

Thanks.

eiriktsarpalis commented 9 years ago

Fixed in 4886670465d8b148a8afdfb6c3fb786d84e60507

I think not. Formats didn't change, only a pickler implementation.

Bananas-Are-Yellow commented 9 years ago

Thanks for making this improvement for me.

I'm quite new to GitHub. How do I try out your fix? Do I have to wait for you to increment the version number and make a new version available in NuGet? I tried downloading the source, editing AssemblyInfo.fs to change to 1.0.6 and running 'build' (I don't really know what I am doing), but there is something in the build script that puts the assembly version number back to 1.0.5.

eiriktsarpalis commented 9 years ago

Yes, the version number is set automatically by FAKE. You could just build it as version 1.0.5 and use it as a drop-in replacement