jsakamoto / ipaddressrange

.NET Class Library for range of IP address, both IPv4 and IPv6.
Mozilla Public License 2.0
368 stars 71 forks source link

optimize Bits methods - remove Expensive linq operations #20

Closed llCorvinSll closed 8 years ago

llCorvinSll commented 8 years ago

Remove Linq operations from some Bits Method. This Avoid creation of IEnumerator objects, cause speedup and memory usage reduction

gregmac commented 8 years ago

How much does this speed these operations up?

Also, -1 for the reformat of code (spaces to tabs, I think?), making it 10x more difficult to read the diff. Don't reformat code at the same time you change something.

jsakamoto commented 8 years ago

@gregmac

How much does this speed these operations up?

I use this https://github.com/jsakamoto/ipaddressrange-benchmark/blob/master/benchMarkApp/Program.cs#L13 C# console app to do benchmark how many time need to creating too many IPAddressRange objects.

I use this https://github.com/jsakamoto/ipaddressrange-benchmark/blob/master/doSpeedBench.ps1#L1 PowerShell script to launch that C# console app 11times and collect results.

I executed this benchmark two patterns about 2,000 objects and 20,000 objects constructing.

My report is here: http://1drv.ms/1PsWt7z

The report tell us that "NO LINQ" approach is more faster around 10msec. than original implements.

And rely on this my report http://1drv.ms/1PsW9G0 , "NO LINQ" approach save around 2MB memory at constructing 20,000 IPAddressRange objects.

But... I feel, it doesn't enough reason to rewrite from "WITH LINQ" to "NO LINQ". In my opinion, "WITH LINQ" approach is more readable, beautiful, and maintenancable.

Exactly, "NO LINQ" save few MB memory and faster few msec in some cases, but I think it is rare case (How many people are there who construct more than thousand objects at once?), so I feel "NO LINQ" approach is overspec.

jsakamoto commented 8 years ago

@llCorvinSll (This is reply for this https://github.com/jsakamoto/ipaddressrange/pull/19#issuecomment-148068270 comment)

2Mb is 12% difference! And and i dont big fan of creating iterators without reason. (bit operations) And, for my taste, linq operations on BITs hard to understand, im prefer to use arrays in this case (: If you dont want to use old way style - reject #20


2Mb is 12% difference!

Yes, that's right.
But, 2MB is around 0.2% of 1GB in OS/PC memory space...

And 2MB is the case that constructing 20,000 IPAddressRange objects.

I executed the benchmark reduce number of objects from 20,000 to 2,000, then saving memory usage was 0.22MB (around 220KB).

Percentage of difference is not small, but absolute value of how many memory usage saving is enough small, I feel.

My ditision at this time, I would like to reject #20...

P.S. I understand that you don't prefer using LINQ in this case. It is right, I think. But I prefer confortable using LINQ in this case for my taste. This is the diference of the taste among with you and me.

llCorvinSll commented 8 years ago

@jsakamoto OK. I suggested it as optional imporvement this request can be closed