zeromq/netmq — pull request #103 by JamesWHurst (2013-10-17 13:53:22+00:00)
Title: Establish NET 3.5 compatibility

Summary of my changes to NetMQ for branch Net35CompatFix:

I created a new Visual Studio 2008 solution, named NetMQ_Vs2008.sln
I went with this naming because that makes it immediately clear which solution is for Visual Studio 2008. I created this within a VMware virtual machine (VM) that does NOT have any other version of Visual Studio, and no other version of the .NET Framework. The latter is important, since that guarrantees I am not accidentally allowing a dependency on another .NET version to creep in. If it builds on this VM, it is .NET 3.5 compatible.

While on the topic of platforms, I also tested it on VMs with Windows XP SP3, Win 7, and Win 8, and Windows Server 2003 and 2008. And on both x64 and x86, to check for any unexpected fragility. It builds, and the unit-tests run successfully on all platforms - with the exception of the pgm tests which I did not run, and the NetMQScheduler which I left out as that particular class may not make sense for 3.5

.NET 3.5 does not have default parameter-values, so I separated those out in some cases into multiple methods, generally having the one call the other.

I added comments mostly only for that code that I myself edited; I don't want to add anything there unless I understand with confidence what it's doing and can compose a decently-informative comment.

Once the comments are fairly complete for this project, I'd like to suggest we use a tool such as SandCastle to automatically produce a decent help file for it and include that within the binary-downloads.

I use ReSharper (plz let me know if you need a copy) and used it for some code-analysis and some of its optimization facilities. For a few of the files, I removed unused using pragmas, plus remove unused references from the project NetMQ_Vs2008.sln

In some cases, I did not want to regress the existing code, so - as in Devices\ThreadedDeviceRunner.cs, I used conditional-compilation and provided two alternative bits of code. I tried to keep this to a minimum, as it does add code-noise to our project, which I am loath to do. In the case of ThreadedDeviceRunner, I left the code that uses Tasks, and added an alternative bit of code that uses Threads instead - as 3.5 did not have Tasks (TPL). TPL simply uses threads itself regardless, but I like its simpler syntax. In this case, I used the ThreadPool to save the execution-time of spinning up a new thread. It is important when using the ThreadPool to know when that is advantageous, and when it's better to just create a new thread.

To implement the conditional-compilation, I used the symbol PRE_4, which means only features that are provided in pre-.NET 4.0 Frameworks. I have found this scheme to be much simpler than trying to use a symbol that means a particular version is intended. If in the future we use C# 5.0/.NET 4.5 features, I recommend we use the symbol PRE_5 to provide compatibility for that platform. For every file in which I use any pragma (in this case, PRE_4 is the only one) I inserted a note at the very top of the file.

Since 3.5 also does not have CancellationTokens, I went with an alternative mechanism for cancelling background threads: Interlock, which is the most efficient mechanism available and maps to the Intel CPU hardware directly. You can see this in use within Monitoring\NetMQMonitor.cs. Here, I created a new method, RequestCancellation, and a property, IsCancellationRequested, to make the actual code clear and simple (with no conditional-compilation), and isolated the actual conditional-compilation sections to within those members. I replicated this pattern several times, identically, where needed. It makes the code even easier to read than it was before.

In file Sockets\SubscriberSocket.cs, I took the liberty of adding the method SubscribeToAll(). I think that is a bit clearer than requiring the user to Subscribe(""). Feel free to remove that of course if it doesn't suit you.

Within the folder Native, with VS2008 I found that some naming conflicts were occurring - as, for example, Poller (we have multiple types with this same name). I guess VS2008 is a bit more particular: I resolved this by appending ".Native" to the namespace of the files within this folder - which is standard C# practice anyway.

Some of the most extensive changes are within file Utils.cs, which I used as a sort of closet to place misc bits of code.
For example, several classes did not have Dispose methods in 3.5. So I added those here, using conditional-compilation as usual such that this code will simply not be included in normal builds (thus no code-bloat). I hate adding this syntactic sugar to the file though; I included comments to make it clear why these needed to exist.

I added a property here, Is64BitProcess, to provide a platform-neutral way to check this. Again, this is so that (where this is used) the messy conditional-compilation can be confined to inside of this property, instead of scattered throughout our code.

Finally, in Utils.cs I added a simplistic substitute for class DnsEndPoint, since that too is unavailable in 3.5

In IOutgoingSocket, I had to break apart that one method, which had 2 parameters with defaults, into two methods. Theoretically one would have to provide more, to cover every possible way of calling it, but I think that having method-overloads with one, and another with two, parameters of the same type, makes it too easy to accidentally specify the wrong parameter, especially since 3.5 does not provide for named-position arguments.

For NetMQFrame, I provided a very-slight optimization in that the Empty property returns a pre-constructed instance, instead of creating a new one with every call.

Another very-small optimization that I did where I saw this: for member variables that are initialized at their point of declaration to their default values, that initialization is redundant and can be removed. For example, in NetMQSocket, m_isClosed is a boolean and will be false by default: there is no need to set it to false explicitly (which results in additional IL-code).

In Poller.cs, at the beginning of method Start, I added a test for CurrentThread.Name being null before setting the Name. If you write a unit-test that uses this and you give your thread a name, that code would crash as this property does not like to be set twice, so - this protects against that.

In Tests\Devices\QueueDeviceTests.cs, and also StreamerDeviceTests.cs, you'll notice that I call StartClient with two arguments instead of one. That's because I removed the parameter-defaults from those methods. Here, I think it is less likely to cause confusion to just require the developer to put two parameters, rather than risk thinking he/she specified one intead of the other (they're both of the same type).

In Tests\zmq\ZMQPollTests.cs, I hate to bloat the code here - but decided to conditionally-compile since the two sections of code are right next to one another, and fairly small, so they can be easily compared visually. With test code, it's more important that it be explicitly transparent what code is executing, as opposed to trying to abstract things.

In places such as MonitorPollTests.cs, it is quite practical to just use the 3.5 version of the code instead of breaking it up via conditional-compilation. However, I specifically wanted to not disturb existing code AMAP; perhaps the author had a strong preference for using Tasks - but if you prefer to simplify that I don't see any harm in reducing this to just use Threads for both versions and eliminating the conditional-compilation here.

Please do note that I did do one edit to the NetMQ.Tests project (file NetMQ.Tests\NetMQ.Tests.csproj) : removed unused references.

I added the Timeout attribute to a few tests, such as PollerTests, as when these fail they can linger forever. This can be important when we set up a continuous-build/continuous-test platform, as in that case you don't want failing tests to hang up your system. Not vital. Just a bit of a convenience.

A rather important note (if you consider solid unit-testing to be as vital as I do): certain of the tests within PollerTests do have a dependency upon the test box having sufficient processing-resources to pass. I tested this on a VM with only one CPU with one core, and that newly-created thread never got to execute in time for these tests. The net result would be intermittent test failures. But not always. TaskManager showed the CPU going 100%. I added a core to that VM, and the tests stopped failing. So, these tests do not necessarily indicate failed functionality if they fail. I added notes to that effect.

Even on a not-so-heavily-loaded VM, test TwoTimers would sometimes fail. The cause was a dependency on the threads executing with too much precision (that is, start up immediately and execute simultaneously). In practice, sometimes a new thread does not fire up instantly. So I adjusted the limit very slightly here - as you'll see by comparing the two files: count2 can be 5 or 6.

I did not test this with Mono - yet. I'll wait for this to be merged in first, before continuing on to that platform.

jh