Enum.IsDefined()

Feb 4, 2008 at 9:54 PM
I was profiling SharpMap 0.9 to see if I could squeeze out a bit more performance (the rendering speed isn't quite what I want--not yet, anyway :), and I noticed that this code in GeometryFromWKB.cs:

public static Geometry Parse(BinaryReader reader)
{
// Get the first Byte in the array. This specifies if the WKB is in
// XDR (big-endian) format of NDR (little-endian) format.
Byte byteOrder = reader.ReadByte();

if (!Enum.IsDefined(typeof(WkbByteOrder), byteOrder))
{
throw new ArgumentException("Byte order not recognized");
}

// Get the type of this geometry.
UInt32 type = readUInt32(reader, (WkbByteOrder)byteOrder);

if (!Enum.IsDefined(typeof(WkbGeometryType), type))
{
throw new ArgumentException("Geometry type not recognized");
}

...I checked, and this code is also in SharpMap 2.0, although how and where it's used, I'm not so sure.

But one thing I'm sure of: this should be reworked to not use Enum.IsDefined, which is a very expensive operation (under the covers, it's doing all sorts of reflection, boxing, etc) and has a definite impact on my data processing perfomance. I changed this code to look something like:

public static Geometry Parse(BinaryReader reader)
{
// Get the first byte in the array. This specifies if the WKB is in
// XDR (big-endian) format of NDR (little-endian) format.
WkbByteOrder byteOrder = (WkbByteOrder)reader.ReadByte();

// Get the type of this geometry.
WKBGeometryType type = (WKBGeometryType)ReadUInt32(reader, byteOrder);

switch(type)
{
case WKBGeometryType.wkbPoint:
return CreateWKBPoint(reader, byteOrder);

...and ReadUInt32() now throws an exception if the read byte is not defined by WkbByteOrder.Xdr, using an if/else if/else statement instead of Enum.IsDefined():

private static uint ReadUInt32(BinaryReader reader, WkbByteOrder byteOrder)
{
if (byteOrder == WkbByteOrder.Xdr)
{
byte[] bytes = BitConverter.GetBytes(reader.ReadUInt32());
Array.Reverse(bytes);
return BitConverter.ToUInt32(bytes, 0);
}
else if( byteOrder == WkbByteOrder.Ndr)
{
return reader.ReadUInt32();
}
else
throw new ArgumentException("Byte order not recognized");
}

...thoughts?

Feb 6, 2008 at 5:24 PM
I found some other performance enhancements in WorldtoMap. The first is a freebie. Change:

System.Drawing.PointF result = new System.Drawing.Point();

...to:

System.Drawing.PointF result = new System.Drawing.PointF();

...before it was performing an unnecessary cast under the covers.

Another thing that gets computed over, and over, and over again are top, left, pixel width, and pixel height. Computing all of these involves a ton of floating point operations. To deal with these, without really making any drastic changes, I added several overloads for WorldtoMap:

public static System.Drawing.PointF WorldtoMap(SharpMap.Geometries.Point p, SharpMap.Map map)
{
System.Drawing.PointF result = new System.Drawing.PointF();
double left, top;
GetLeftAndTop(map, out left, out top);
return WorldtoMap(p, map, left, top, map.PixelWidth, map.PixelHeight);
}

public static System.Drawing.PointF WorldtoMap(SharpMap.Geometries.Point p, SharpMap.Map map, double left, double top, double pixelWidth, double pixelHeight)
{
System.Drawing.PointF result = new System.Drawing.PointF();
result.X = (float)((p.X - left) / pixelWidth);
result.Y = (float)((top - p.Y) / pixelHeight);
return result;
}

public static void GetLeftAndTop(SharpMap.Map map, out double left, out double top)
{
double Height = (map.Zoom * map.Size.Height) / map.Size.Width;
left = map.Center.X - map.Zoom * 0.5;
top = map.Center.Y + Height * 0.5 * map.PixelAspectRatio;
}

...and then, wherever WorldtoMap is being called in a loop, compute top and left with GetLeftAndTop, and pass that and pixel width/height in to overloaded constructor. Doing this cuts the amount of time spent in WorldtoMap drastically.
Coordinator
Feb 8, 2008 at 12:16 AM
This discussion has been copied to a work item. Click here to go to the work item and continue the discussion.