Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.Net 3.5 support #52

Closed
varon opened this issue Jul 1, 2015 · 21 comments
Closed

.Net 3.5 support #52

varon opened this issue Jul 1, 2015 · 21 comments

Comments

@varon
Copy link
Contributor

varon commented Jul 1, 2015

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
Copy link
Member

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

@eiriktsarpalis
Copy link
Member

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
Copy link
Contributor Author

varon commented Jul 3, 2015

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

@eiriktsarpalis
Copy link
Member

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
#52 (comment).

@varon
Copy link
Contributor Author

varon commented Jul 3, 2015

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:

  • ThreadLocal<'T>
  • bigInt.ToByteArray()
  • System.Numerics.BigInteger(data) -> bigint
  • Guid.Parse
  • RuntimeHelpers.EnsureSufficientExecutionStack()

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

@varon
Copy link
Contributor Author

varon commented Jul 3, 2015

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
Copy link
Member

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:

  • ThreadLocal could be replaced by ThreadStaticAttribute.
  • BigInteger is not available in net35, so all its serialization functionality can be excluded in net35.
  • Guid.Parse can be replaced by the corresponding constructor.
  • EnsureSufficientExecutionStack dependency can be removed by disabling the PROTECT_STACK_OVERFLOWS compile flag.

@eiriktsarpalis
Copy link
Member

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
Copy link
Contributor Author

varon commented Jul 4, 2015

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor Author

varon commented Jul 5, 2015

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
Copy link
Contributor Author

varon commented Jul 5, 2015

The net35 build works in Unity:

  • It loads correctly
  • I was able to call serialization functions from within Unity
  • I was able to interop by sending 4.5 binary serialized data to 3.5

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
Copy link
Member

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
Copy link
Contributor Author

varon commented Jul 7, 2015

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:

  • Clone the repository from scratch.
  • Swap the solution configuration to Release-NET35
  • Build solution

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
Copy link
Contributor Author

varon commented Jul 7, 2015

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

@varon
Copy link
Contributor Author

varon commented Jul 7, 2015

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
Copy link
Member

Fixed in 0128169.

@varon
Copy link
Contributor Author

varon commented Jul 7, 2015

Thanks!

@eiriktsarpalis eiriktsarpalis reopened this Jul 9, 2015
@eiriktsarpalis
Copy link
Member

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
Copy link
Member

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

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

No branches or pull requests

2 participants