NimaAra / Easy.MessageHub

No need for .NET Events! A thread-safe, high performance & easy to use cross platform implementation of the Event Aggregator Pattern.
MIT License
258 stars 41 forks source link

Migrated from .NET to Portable (Profile7) #4

Closed ravensorb closed 7 years ago

ravensorb commented 8 years ago

Updated code Updated nusepc file (note, building nuspec now can take a Configuration Parameter to build release or debug)

NimaAra commented 8 years ago

Thanks for the PR, I just noticed you have modified the TargetFramework from 4.0 to 4.5.*, the library needs to support 4.0 at the minimum.

I would also prefer .NETStandard as opposed to PCL.

ravensorb commented 8 years ago

Ok for supporting .net 4.0 we could do multiple project and handle things ib the nusoec. The .netstandard is tougher as there is a lot that doesn't work with that yet. The profile I picked is technically .netstandard1.1 if I remember correctly.

NimaAra commented 8 years ago

Okay with the PCL for now and okay with the multiple projects... would we be able to put all of that into a single NuGet?

ravensorb commented 8 years ago

Absolutely -- I do it all the time... I'll add support for the .NET 4 project and update the nuspec file

NimaAra commented 8 years ago

Okay thanks, let's do that then.

On 6 August 2016 11:44:52 BST, Shawn notifications@github.com wrote:

Absolutely -- I do it all the time... I'll add support for the .NET 4 project and update the nuspec file


You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/NimaAra/Easy.MessageHub/pull/4#issuecomment-238017559

This email was sent using a mobile device, please excuse my brevity.


Can't remember your password? Do you need a strong and secure password? Use Password manager! It stores your passwords & protects your account. Check it out at http://mysecurelogon.com/manager

ravensorb commented 8 years ago

Ok, this update will support both .NET4 and the PCL profiles. I also made a small tweak to the folder structure and added support for command line builds via (cakebuild)[http://cakebuild.net/] -- hopefully that is ok with you :) All that is needed now is to update the settings file with a nuget api key and you can build,test,package, and deploy from the command line

NimaAra commented 8 years ago

Great thanks for the quick fix, I'm going to be on a plane soon with no connectivity will integrate and release as soon as I get the chance.

On 6 August 2016 12:51:38 BST, Shawn notifications@github.com wrote:

Ok, this update will support both .NET4 and the PCL profiles. I also made a small tweak to the folder structure and added support for command line builds via (cakebuild)[http://cakebuild.net/] -- hopefully that is ok with you :) All that is needed now is to update the settings file with a nuget api key and you can build,test,package, and deploy from the command line


You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/NimaAra/Easy.MessageHub/pull/4#issuecomment-238019670

This email was sent using a mobile device, please excuse my brevity.


FREE 3D EARTH SCREENSAVER - Watch the Earth right on your desktop! Check it out at http://www.inbox.com/earth

NimaAra commented 8 years ago

I just had a chance to look at the changes, they look really good and I like the new setup however when running the benchmark I realized the performance has dropped from 180million operations per second to around 90 million.

I cannot understand why the changes have resulted in such a drastic regression in performance. But it needs to be investigated.

Would you be able to run the benchmarks on your side and let me know what you get (comparing master and the branch)

Thanks

ravensorb commented 8 years ago

Odd there were only two lines of code that changed and there are now #if statements around them so the .net4 version will compile. Can you run the benchmark against the release build of them both and see if the one line is the diff?

NimaAra commented 8 years ago

It's the inclusion of the using System.Reflection; regardless of the compilation flag it results in the poor performance. The namespace is required to satisfy the following line:

if (!subscription.Type.GetTypeInfo().IsAssignableFrom(msgType.GetTypeInfo())) { continue; }

Need to run some profiling to figure out:

xleon commented 7 years ago

I´d like to use this library with Xamarin. Any news on this PR?

NimaAra commented 7 years ago

@xleon I have not had time to take a look at the issues mentioned above and frankly I do not think I will be picking this up any time soon as I am quite busy with other projects with higher priority.

I have marked it as help wanted for anyone willing to address the issues.

xleon commented 7 years ago

I don´t think performance is an issue for most developments. It could matter if you send hundreds of messages per second. In mobile development at least you will probably send just one message at a time. Why not releasing a nuget with the current @ravensorb PR? If someone needs better performance, the older version can be used.

NimaAra commented 7 years ago

@xleon I am sorry but that is not acceptable. Performance has always been and will always remain a big feature in anything that I put out there.

This PR causes a significant regression (50% drop) in performance and we have many low-latency/high-frequency components relying on this library to meet their SLAs; I cannot let this regression slip into a NuGet release knowing there is a problem.

xleon commented 7 years ago

@NimaAra That´s pretty understandable. Thanks anyway

NimaAra commented 7 years ago

As of v3.0.0 the library supports Net 3.5 and netstandard1.0.