mbraceproject / FsPickler

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

.Net 3.5 support #52

Closed varon closed 8 years ago

varon commented 9 years ago

FsPickler has great potential for usage in Unity3D, as the serialization is fast enough to offer some performance benefits to games.

Unfortunately, it doesn't compile for .Net 3.5 at the moment, which is the version of the framework that Unity3D works with. (although it's actually some heavily modified mono version)

If there's any chance at supporting this version, I'm sure FsPickler would see some not-insignificant adoption among Unity3D users, especially if we could put out a Nuget package for it.

I'm currently doing a cursory look at what's going to be involved in porting this to 3.5, as I'd really like to use it.

I'll update this issue with my findings, but I'd appreciate input on this from the maintainers.

eiriktsarpalis commented 9 years ago

I'd be happy to include a net35 build of FsPickler in its nuget package, let me know if you need any help.

eiriktsarpalis commented 9 years ago

Ok so I tried building with net35 and there were quite a few errors, most of which relate to the lack of concurrent collections. These can be obtained using the TPL backport. The other errors seem more or less fixable.

varon commented 9 years ago

I'll give this a bash this evening. Are you okay with adding a dependency on the TPL backport?

eiriktsarpalis commented 9 years ago

I think I would rather have it as a separate package. On Sat, 4 Jul 2015 at 00:50 varon notifications@github.com wrote:

I'll give this a bash this evening. Are you okay with adding a dependency on the TPL backport?

— Reply to this email directly or view it on GitHub https://github.com/nessos/FsPickler/issues/52#issuecomment-118431918.

varon commented 9 years ago

Could you elaborate on what you mean by 'separate package' ? I'm only referring to the specific 3.5 build adding it as a dependency.

I have a few questions about the port:

How do we want to handle the conditional compilation flags for NET35 and NET40? It''s pretty ugly looking if we define them separately:

#if NET35
    do if leaveOpen then raise <| new NotSupportedException("'leaveOpen' not supported in .NET versions < 4.5.")
    let bw = new BinaryWriter(stream, encoding)
#else
#if NET40
    do if leaveOpen then raise <| new NotSupportedException("'leaveOpen' not supported in .NET versions < 4.5.")
    let bw = new BinaryWriter(stream, encoding)
#else
    let bw = new BinaryWriter(stream, encoding, leaveOpen)
#endif
#endif

Unfortunately, we can't use #if NET35 || NET40 due to compiler restrictions, so I'd propose defining a custom flag for either NET35 or 40 to encapsulate this condition:

#if NET35OR40
    do if leaveOpen then raise <| new NotSupportedException("'leaveOpen' not supported in .NET 40.")
    let bw = new BinaryWriter(stream, encoding)
#else
    let bw = new BinaryWriter(stream, encoding, leaveOpen)
#endif

Otherwise, from what I can tell, there's a few non-obvious functions that will need a replacement in 3.5:

If you have any input on these, please let me know.

varon commented 9 years ago

SerializationInfo.ObjectType also needs a replacement. Based on the source for it, it doesn't seem to expose this apart from this property. Perhaps we'll need to write a function that utilizes reflection, or perhaps preferably, our own type that implements the functionality of the 4.0 version of the class.

I'll halt here for now until I can get some feedback - I'd like to make sure my changes line up with your plans for this, rather than haphazardly changing while being unfamiliar with the codebase and style.

You can view my progress here: https://github.com/varon/FsPickler/tree/dotnet35

eiriktsarpalis commented 9 years ago

So, the current nuget package bundles bother net45 and net40 builds. Given that a net35 build would introduce a redundant dependency for all the other, I would rather have it as a separate nuget package, i.e. FsPickler.Net35.

The conditional flags could be renamed to something like NET40_OR_OLDER and NET35_OR_OLDER. This would do away with all the redundant code. I know that F# 4.0 incorporates boolean operators in its #if conditionals, so this could be possibly changed back in the future.

Rergarding your other points:

eiriktsarpalis commented 9 years ago

The SerializationInfo.ObjectType issue is tricky. We can't rely on reflection, since the name of the internal fields are highly implementation sensitive, and usually vary depending on the framework implementation and version. We also cannot replace the type, since this must be passed to user-defined ISerializable implementations. It needs considerable refactoring, and possibly removal of some functionality.

varon commented 9 years ago

Thanks for the answers.

With regards to SerializationInfo.ObjectType, Perhaps we might want to use some new record which contains a SerializationInfo and the Type itself - we can then use the new type in FsPickler (with presumably minimal refactoring) and still be able to pass in the SerializationInfo into the ISerializable implementations.

eiriktsarpalis commented 9 years ago

No, SerializationInfo.ObjectType containes a type that might have been set by the class implementor at runtime through SerializationInfo.SetType. After some investigation I chose not to support a certain class of serialization patterns for net35.

I had a look at your branch, it has a few problems in that it has overwritten build configuration for the other target frameworks. Unfortunately, the Visual Studio UI tools do not support targeting multiple frameworks across different build configurations; it needs custom editing of the msbuild files. I created a branch of my own that does this; the net35 build can be accessed by choosing the RELEASE-NET35 build configuration. It seems to be compiling fine at the moment, more work needs to be done to make tests and the C# wrapper library build.

eiriktsarpalis commented 9 years ago

I just created a PR that resolves this. Please review, let me know if this works with mono/unity before I can merge it.

varon commented 9 years ago

Thanks sincerely for the effort! I'm hugely appreciative and awed by the unbelievably helpful and quick response.

I'll review it in Unity and report back.

While I do so, are there any gotchas that one might encounter? Specifically I wondered if the binary serializer would work correctly between different .Net versions?

It might be worth documenting this in addition to the missing support for bigInt and stack overflow protection.

varon commented 9 years ago

The net35 build works in Unity:

I wasn't able to build from VS, although I don't suppose it's that significant if there's an existing Nuget package for it.

eiriktsarpalis commented 9 years ago

In general, I would not recommend using FsPickler for communicating between different versions of runtimes. This is because many of the types use field-based serialization, which is highly implementation sensitive. Also, the library has not been designed with version tolerance in mind.

What was exactly the build error you were getting? What version of VS are you using?

varon commented 9 years ago

I'm using VS2013 Community.

It seemed to build perfectly once I'd run the build scripts at least once, but on a fresh clone it seems to fail:

Repro steps:

I receive the following errors:

Warning 1   The primary reference "System.Threading", which is a framework assembly, could not be resolved in the currently targeted framework. ".NETFramework,Version=v3.5". To resolve this problem, either remove the reference "System.Threading" or retarget your application to a framework version which contains "System.Threading".    C:\Program Files (x86)\MSBuild\12.0\bin\Microsoft.Common.CurrentVersion.targets 1697    5   FsPickler
Error   2   The type 'ThreadLocal' is not defined   C:\Work\temp\FsPickler\src\FsPickler\Format\BinaryFormat.fs 53  22  FsPickler
Error   3   Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\Format\BinaryFormat.fs 58  19  FsPickler
Error   4   Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\Format\BinaryFormat.fs 74  19  FsPickler
Error   5   This expression was expected to have type
    bool    
but here has type
    obj C:\Work\temp\FsPickler\src\FsPickler\FsPickler\FsPickler.fs 59  20  FsPickler
Error   6   The namespace 'Concurrent' is not defined   C:\Work\temp\FsPickler\src\FsPickler\Pickler\ReflectionCache.fs 10  25  FsPickler
Error   7   The type 'ConcurrentDictionary' is not defined  C:\Work\temp\FsPickler\src\FsPickler\Pickler\ReflectionCache.fs 296 33  FsPickler
Error   8   Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\Pickler\ReflectionCache.fs 314 15  FsPickler
Error   9   Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\Pickler\ReflectionCache.fs 315 19  FsPickler
Error   10  The namespace 'Concurrent' is not defined   C:\Work\temp\FsPickler\src\FsPickler\Pickler\ReflectionPicklers.fs  6   25  FsPickler
Error   11  The namespace 'Concurrent' is not defined   C:\Work\temp\FsPickler\src\FsPickler\PicklerGeneration\PicklerCache.fs  9   25  FsPickler
Error   12  The type 'ConcurrentDictionary' is not defined  C:\Work\temp\FsPickler\src\FsPickler\PicklerGeneration\PicklerCache.fs  37  20  FsPickler
Error   13  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\PicklerGeneration\PicklerCache.fs  44  33  FsPickler
Error   14  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\PicklerGeneration\PicklerCache.fs  48  66  FsPickler
Error   15  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\PicklerGeneration\PicklerCache.fs  53  21  FsPickler
Error   16  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\PicklerGeneration\PicklerCache.fs  75  38  FsPickler
Error   17  The namespace 'Concurrent' is not defined   C:\Work\temp\FsPickler\src\FsPickler\PicklerGeneration\PluginRegistry.fs    4   25  FsPickler
Error   18  The type 'ConcurrentDictionary' is not defined  C:\Work\temp\FsPickler\src\FsPickler\PicklerGeneration\PluginRegistry.fs    15  31  FsPickler
Error   19  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\PicklerGeneration\PluginRegistry.fs    18  9   FsPickler
Error   20  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\PicklerGeneration\PluginRegistry.fs    21  9   FsPickler
Error   21  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\PicklerGeneration\PluginRegistry.fs    24  15  FsPickler
Error   22  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\PicklerGeneration\PluginRegistry.fs    29  15  FsPickler
Error   23  The operator 'expr.[idx]' has been used on an object of indeterminate type based on information prior to this program point. Consider adding further type constraints   C:\Work\temp\FsPickler\src\FsPickler\PicklerGeneration\PluginRegistry.fs    34  15  FsPickler
Error   24  The namespace 'Concurrent' is not defined   C:\Work\temp\FsPickler\src\FsPickler\Utils\Utils.fs 10  29  FsPickler
Error   25  The type 'ConcurrentDictionary' is not defined  C:\Work\temp\FsPickler\src\FsPickler\Utils\Utils.fs 121 25  FsPickler
Error   26  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\Utils\Utils.fs 122 18  FsPickler
Error   27  The type 'ConcurrentDictionary' is not defined  C:\Work\temp\FsPickler\src\FsPickler\Utils\Utils.fs 127 25  FsPickler
Error   28  The type 'ConcurrentDictionary' is not defined  C:\Work\temp\FsPickler\src\FsPickler\Utils\Utils.fs 128 26  FsPickler
Error   29  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\Utils\Utils.fs 132 25  FsPickler
Error   30  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\Utils\Utils.fs 136 17  FsPickler
Error   31  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\Utils\Utils.fs 139 17  FsPickler
Error   32  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\Utils\Utils.fs 145 25  FsPickler
Error   33  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\Utils\Utils.fs 149 17  FsPickler
Error   34  Lookup on object of indeterminate type based on information prior to this program point. A type annotation may be needed prior to this program point to constrain the type of the object. This may allow the lookup to be resolved. C:\Work\temp\FsPickler\src\FsPickler\Utils\Utils.fs 152 17  FsPickler
Error   35  Assembly reference 'C:\Work\temp\FsPickler\bin\net35\FsPickler.dll' was not found or is invalid FSC 1   1   FsPickler.Json
Error   36  Metadata file 'C:\Work\temp\FsPickler\bin\net35\FsPickler.dll' could not be found   C:\Work\temp\FsPickler\src\FsPickler.CSharp\CSC FsPickler.CSharp
Error   37  Metadata file 'C:\Work\temp\FsPickler\bin\net35\FsPickler.Json.dll' could not be found  C:\Work\temp\FsPickler\src\FsPickler.CSharp\CSC FsPickler.CSharp
Error   38  Assembly reference 'C:\Work\temp\FsPickler\bin\net35\FsPickler.dll' was not found or is invalid FSC 1   1   FsPickler.PerfTests
Error   39  Assembly reference 'C:\Work\temp\FsPickler\bin\net35\FsPickler.Json.dll' was not found or is invalid    FSC 1   1   FsPickler.PerfTests
Error   40  Assembly reference 'C:\Work\temp\FsPickler\bin\net35\FsPickler.dll' was not found or is invalid FSC 1   1   FsPickler.Tests
Error   41  Assembly reference 'C:\Work\temp\FsPickler\bin\net35\FsPickler.Json.dll' was not found or is invalid    FSC 1   1   FsPickler.Tests
varon commented 9 years ago

Obviously this seems to be a NuGet error from within VS - Probably it isn't referencing the System.Threading package correctly.

varon commented 9 years ago

Please disregard my report of a build error. This seems to be standard behaviour for the F# Scaffold Project.

From what I understand, one needs to use Paket first to resolve dependencies, and this isn't included in the build process from Visual Studio. It might be worth documenting this, because it was non-obvious to me why I didn't have a one-step build from the IDE.

eiriktsarpalis commented 9 years ago

Fixed in 012816963cbe021545fa7c34f2dce5167898a2c1.

varon commented 9 years ago

Thanks!

eiriktsarpalis commented 9 years ago

The nuget package version 1.2.12 now incorporates net35 builds. However, this has not been extended to the FsPickler.CSharp package because of a few issues in the FSharp.Core nuget package. This why I'll keep this open for now.

eiriktsarpalis commented 8 years ago

Closing, as the FsPickler.CSharp problem has been resolved.