EDDiscovery / BaseUtilities

C# Base utilities set
1 stars 8 forks source link

Speed up Point3D.X/Y/Z get/set #14

Closed EoD closed 4 years ago

EoD commented 5 years ago

This PR improves the average time of the following loop by around 10%.

Here is a small benchmark I wrote to accompany my above findings: https://github.com/EoD/EDDBenchmarks/blob/bc025004ea492b2cf01343baa30e2a8c1d0960fc/EDDBenchmarks/Point3DDistanceBenchmark.cs#L47-L70 Original:

Generating random Point3D list of size 25000
List generated, duration: 0.003s

Calculating distances
Total: 3.381957E+07, duration: 6.334s
Total: 3.381957E+07, duration: 6.198s
Total: 3.381957E+07, duration: 6.194s
Total: 3.381957E+07, duration: 6.185s
Total: 3.381957E+07, duration: 6.210s

With the patch around 10% faster

Generating random Point3D list of size 25000
List generated, duration: 0.002s

Calculating distances
Total: 3.38295E+07, duration: 5.646s
Total: 3.38295E+07, duration: 5.616s
Total: 3.38295E+07, duration: 5.614s
Total: 3.38295E+07, duration: 5.614s
Total: 3.38295E+07, duration: 5.641s

EDIT: There is also a major performance difference with debugging enabled. The difference is >400%. These are screenshots from the dotTrace Profiler:

Original: dotTrace_before

After the changes: dotTrace_after

robbyxp1 commented 5 years ago

I'm surprised this did anything. Did you run it in release mode to see if there was a difference?

Also your making the setters private, thus changing the interface..

EoD commented 5 years ago

@robbyxp1 you are right, in Release builds there is no measurable difference. I am not sure how to inspect this.

I also tried the same change with Vector3D and it had literally the same impact. Added the respective commit.

I dropped the private bits to keep the API.

Here are the IL-Viewer output of Debug vs Release. It looks identical before and after the commits of this PR:

Debug:

  .method public hidebysig specialname instance float64
    get_X() cil managed
  {
    .maxstack 2
    .locals init (
      [0] float64 V_0
    )

    // [62 58 - 62 59]
    IL_0000: nop

    // [62 60 - 62 83]
    IL_0001: ldarg.0      // this
    IL_0002: ldfld        float64[] EMK.LightGeometry.Point3D::_Coordinates
    IL_0007: ldc.i4.0
    IL_0008: ldelem.r8
    IL_0009: stloc.0      // V_0
    IL_000a: br.s         IL_000c

    // [62 84 - 62 85]
    IL_000c: ldloc.0      // V_0
    IL_000d: ret

  } // end of method Point3D::get_X

Release:

  .method public hidebysig specialname instance float64
    get_X() cil managed
  {
    .maxstack 8

    // [64 11 - 64 26]
    IL_0000: ldarg.0      // this
    IL_0001: ldfld        float64[] EMK.LightGeometry.Point3D::_Coordinates
    IL_0006: ldc.i4.0
    IL_0007: ldelem.r8
    IL_0008: ret

  } // end of method Point3D::get_X
iainross commented 5 years ago

foo { get { return bar; } } and foo { get => bar; } are equivalent, the latter is just a bit of syntactic sugar to save us some key strokes, the IL is meant to be identical.

EoD commented 5 years ago

@iainross yeah, I was just trying to find why it makes a performance difference in the Debug build. A random guess: internally the new syntax is not a function call and this one function call causes some overhead in the debug build?

robbyxp1 commented 4 years ago

I'll bring this in later, when we go for 11.2