Performance Quiz #11: Ten Questions on Value-Based Programming
Some of you have probably heard one or more of my talks, or read the annotations I made in the Design Guidelines. If you have then you already know that I don't always agree with every suggested guideline. Especially not in every context. It's probably fair to say that one of the greatest areas of disagreement has to do with the handling of value types and the prevelance of properties vs. fields in the framework.
Generally, my feeling is that properties are highly overrated and fields terribly under-utilized. Did I mention that not everyone agrees with this position? :)
So suffice to say that the guidelines are still good guidelines but of course they are only that, and so you should know when it might be a good time to consider disregarding some of them. Today I wanted to write a motivational example for you that shows the kind of situation that might call for an approach that is more "old-school" if you like.
To try to make it more real, I've cooked it up in terms of some graphics library primitives that might happen in a real graphics library that was dealing with objects made up of 4 side polygons (quads) with some basic texturing. Naturally this example isn't complete but hopefully there is enough there that you could imagine what the other primitives might look like.
Below is the code written in the style that I would suggest, following are some key questions which are intended to suggest the origin of my thinking on some of the more important points.
// typical 3d coordinate
struct Point3d
{
public double x;
public double y;
public double z;
// handy math methods
}
// projected cooridinates in the text space
struct Point2d
{
public double u;
public double v;
// handy math methods
}
// useful for normals etc.
struct Vector3d
{
public double dx;
public double dy;
public double dz;
// handy vector math methods
}
// a single vertex, with location, and normal
struct Vertex
{
public Point3d location;
public Point2d uvmap;
public Vector3d normal;
// manipulation methods
}
struct Quad
{
// vertex indices within a mesh
public int corner1;
public int corner2;
public int corner3;
public int corner4;
}
// at last a reference type
class MeshSection
{
private Vertex[] vertices;
private Quad[] quads;
private TextureMap texture; // defined elsewhere
// assorted methods that accept arrays of vertices and quads
// and insert them into the structure and things like that
}
Question #1: Point3d is a struct, not a class. Why?
Question #2: Point3d.x is a field, not a property. Why?
Question #3: Vertex is a struct, not a class. Why? Same reason as #1?
Question #4: Vertex.location is a field, not a property. Why? Same reason as #2?
Question #5: Quad has no methods. Why?
Question #6: MeshSection is a class with private members. Why?
Question #7: Why do I suggest that MeshSection methods accept arrays of Vertices, Quads, and the like rather than singletons?
Question #8: No mention is made of synchronization here at all, is that just an oversight?
Question #9: How useful is the "foreach" construct likely to be when working with arrays of vertices or quads etc?
Question #10: How many rules did I break? :)
Full Disclosure:
Of course Brad and Krzysztof (the principle authors of the guidelines) both understand that the guidelines need to be broken sometimes. Sometimes we disagree when the bar for breaking them has been reached but I think that's actually a good thing because that kind of tension makes for good professional growth for everyone and better discourse for our customers.
Whenever you decide to go off the recommended path it's still good to be familiar with the guidelines in the area because they can give you some valuable information about what the consequences might be so that you can make an informed decision. Like in this parctiular case you might learn about some serialization issues you'll have to deal with since properties were not chosen.
Try to answer yourself before you peek at the comments, there's good thoughts down there. :)
Update: I posted my solution here.
Comments
- Anonymous
September 01, 2006
Hi Rico,
these are really tough questions and I do not have the answer to all of them.
1. Point3D has 16 bytes as struct (double has 6 bytes). A class even with no data would already allocate 16 bytes (sync block, ...)
2. Point3D.x vs a property get method does look different in disassembled (Release) code:
p3d1.x += 10;
00000159 fld dword ptr ds:[00CD1368h]
0000015f fadd qword ptr [ebp-20h]
00000162 fstp qword ptr [ebp-20h]
p3d1.X += 10;
00000165 lea eax,[ebp-20h]
00000168 fld qword ptr [eax]
0000016a fadd dword ptr ds:[00CD1370h]
00000170 fstp qword ptr [ebp-20h]
There seems to be an additional overhead of loading the address of the structure whereas the field access can spare the "lea eax,[ebp-20h]" instruction.
3. Vertex is 64 bytes big. Size does matter but it is not the same reason. Structs are allocated at the stack. Since we expect maaaany Vertexes in a 3D application we need to minimize the GC pressure. Structs are perfect for this because they do not trigger any GC collections.
4. This is pure matter of usability
Try this
Vertex v = new Vertex();
v.location.x = 1;
v.Location.x = 2;
where Location is a property. You will get a compile time error that you cannot assign the property Location because it is not a variable (value types remember)?
5. I do not know the answer to this one. Perhaps it is used only as index value anyway.
6. Private members can help the JITer to remove array bounds checks when accessing these internal arrays. To ensure thread safety could be another reason.
7. Since we are in the volume processing businness calling a function for every vertex would be desastrous. Imagine you already have a a vertex array if you put a value out of the array it is copied (64 bytes) and then it is copied again to call the function with the second copy.
void AddVertex(Vertex v)
{ ... }
Vertex [] vs = ...;
AddVertex(vs[0]); // Copy out of array, and copy again to call the function!
8. Structs are not objects and have not sync block. Therefore you cannot lock on the structs. But this is not necessary anyway because the second free threading principle (after immutability) is isolation where you simply work with thread local copy of the data. Since structs can be simply copied by assigning it to another variable we can ensure isolation quite easily.
9. A foreach construct is quite useless since you do normally alter the data in your for loop. But since you work with a copy you need to store the changed data back into the same array location it was copied from. You will have a hard time doing this within a foreach loop.
10. Countless times ;-)
It was fun I hope not all my answers are correct ;-)
Yours,
Alois Kraus
- Anonymous
September 02, 2006
The comment has been removed - Anonymous
September 02, 2006
I am as Simon, not so expert, but I will try to answer from what make sense to me
1 - struct is more efficient than class, with allocating, memory releasing...etc
and for a graphic application that uses tons of points, it is very important to consider the performance for such an essential entity like point, so making it as struct instead of class will make sense.
Beside we don't want to override the normal behavior of point.
2 - making the x as property, will be translated as a method call in the native code, even it is an inline function, but still is a method that exist in the heap which will be called to retreive the data, where public field will be accessed directly from the stack.
3 - I think is the same as #1, and I don't find other reason.
6 - MeshSection is a class because there is possibility to override it. - Anonymous
September 03, 2006
Question #1: Point3d is a struct, not a class. Why?
I've two reasons, you will use a Point3D (and a vector) like a math primitive, you can add, increment some coordinate etc.. but you don't want to worry about where this point come from, and the consecuences of any change in other places: has no identity.
The second reason is that, usally, it will arranged in a array with other points3d, if you choose struct you're not just avoiding one level indirection and one vtable pointer, the big trick is that every point will be allocated continuously in the heap (an array is allwais in the heap) not spreaded and accesible throught pointers. This locality very caché candy.
Question #2: Point3d.x is a field, not a property. Why?
Simplifies increments (p.x+=1), allow passing by ref ( Switch(ref p.x, ref p.y); ). Avoid a call to the getter / setter that will do nothing in any inmaginable point structure.
Question #3: Vertex is a struct, not a class. Why? Same reason as #1?
Only the second answer for the first question: theres is no values semantics or identity issues here, but if you declare as a struct you will get a really compacted Vertex[]. However, be carefull with Vertex asignations 'couse is too big.
Question #4: Vertex.location is a field, not a property. Why? Same reason as #2?
Two reasons, the usability problem Rico saids ( v.position.x++ ) and a performance problem: avoid copying the point structure each time Posicion is accesed.
Question #5: Quad has no methods. Why?
You will use a common 'trick' in mesh classes, reusing a vertex in each neighboring face. The Quad is used then to annotate the implied vertex, but not own the vertex 'couse they're shared, and encapsulation should be rised to the Mesh level.
Question #6: MeshSection is a class with private members. Why?
It's whe first one that can be used like a normal class, with primitives to modify the mesh in a encapsulated way.
Question #7: Why do I suggest that MeshSection methods accept arrays of Vertices, Quads, and the like rather than singletons?
I don't understand singletons here, but uses Arrays instead of any other data structure because it's the only that really can change a position without copyiing the content once in the getter and again in the setter (even in a Indexer).
Question #8: No mention is made of synchronization here at all, is that just an oversight?
Is useless do syncronization jobs at Vertex and Point structures, just as in Int
The Mesh is, in fact, something like a collection. The same reason to not doing any sincronization in the List.Add method is applicable for the Mesh methods: syncronized regions have more sense in the complete algorithm that uses a Mehs.
Question #9: How useful is the "foreach" construct likely to be when working with arrays of vertices or quads etc?
Very useless: You can't modiffy an element 'couse is a struct and is copied to the foreach variable, and you can't assign a new element in a forech too. It's only usefull for readonly loops and i think it will be a bit slower.
Question #10: How many rules did I break? :)
Public fields in Point3D, Point2D, Vector3D and Vertex. Vertex is a big struct. Lack of encapsultation in Quad. Use of foreach whenever possible...
I thing the only remaing one it's to plague a for loop of breaks and continues anywhere :)
Anyway, I agree any single rule you've broken, but I won't reapeat that in a inquisition judgment :D - Anonymous
September 03, 2006
Is there a link to the annotated design guidelines? - Anonymous
September 04, 2006
http://www.amazon.com/exec/obidos/tg/detail/-/0321246756?v=glance
:-D - Anonymous
September 04, 2006
Question #10: How many rules did I break? :)
None! All your types are internal and so the design guidelines don’t apply :-) - Anonymous
September 04, 2006
I haven't looked at other answers :) . Here goes:
Question #1: Point3d is a struct, not a class. Why?
A: To be able to allocate/deallocate a block of them fast using arrays (new Point3d[]) and not to create GC preasure because you generaly have lots of them which could mean lots of references wich isn't very good.
Question #2: Point3d.x is a field, not a property. Why?
A: Because the CLR inlines a max of 4 consecutive functions and to give a chance to inline something more meaningful than a simple get/set accessor wich doesn't do anything else
Question #3: Vertex is a struct, not a class. Why? Same reason as #1?
A: Same reason as #1
Question #4: Vertex.location is a field, not a property. Why? Same reason as #2?
A: Yes same reason and also to avoind unnecesary copying of the location when accessing it's fields (Vertex.location.x)
Question #5: Quad has no methods. Why?
A: Good question! Why? For virtual methods is undestandable but for nonvirth methods I don't see any obvious reason.
Question #6: MeshSection is a class with private members. Why?
A: To prevent storing a reference to any array from it's member fields wich might lead to bugs when arrays are recreated.
Question #7: Why do I suggest that MeshSection methods accept arrays of Vertices, Quads, and the like rather than singletons?
A: I think the JIT will place the parameters in processor registers rather than stack when using arrays as parameters because of the size of the structures.
Question #8: No mention is made of synchronization here at all, is that just an oversight?
A: Sincronization would be overkill, realy :) ... How would someone do if if would need to retrive 1000$ from a safe in 1$ bills and would need to lock/unlock the safe for every bill.
Question #9: How useful is the "foreach" construct likely to be when working with arrays of vertices or quads etc?
Certainly less useful because it would retrieve copies of the structures not allowing you to modify the structures directly in the array.
Question #10: How many rules did I break? :)
A: The first one : "Don't break design rules!" :)
Now onto reading the comments ... let's see how many stupid things I've said :-S - Anonymous
September 04, 2006
- AFAIK foreach can be useful because of compiler optimization: the length of the array is determined only once. Many developers write
for (int i = 0; i < arr.Length; ++i)
, i.e. the length is checked for every loop. This cannot happen if you use foreach.
- Anonymous
September 04, 2006
Thomas,
the JITer is very clever to recognize a for loop and does optimize it already. The Length property is only checked once (at least in release mode with optimzations turned on).
The following code
Char [] cArr = new Char[20];
for(int i=0;i<cArr.Length;i++)
{
cArr[i] = 'a';
}
becomes after JITing:
Char [] cArr = new Char[20];
00000000 push edi
00000001 push esi
00000002 push ebx
00000003 mov edx,14h
00000008 mov ecx,111300Ah
0000000d call FBCAEDE8
00000012 mov ecx,eax
for(int i=0;i<cArr.Length;i++)
00000014 xor edx,edx
00000016 mov eax,dword ptr [ecx+4] // Store array length in eax
00000019 test eax,eax
0000001b jle 0000002B // if array length is zero exit loop
{
cArr[i] = 'a';
0000001d mov word ptr [ecx+edx*2+8],61h
for(int i=0;i<cArr.Length;i++)
00000024 add edx,1 // increment i
00000027 cmp eax,edx // compare length in edx against our running index which is in eax
00000029 jg 0000001D // continue until i == cArr.Length
}
To be fair both versions (foreach and for(...) ) are highly optimized. But foreach has slightly more overhead.
Yours,
Alois Kraus - Anonymous
September 04, 2006
It has been a while since I bothered you about this. But this article is practically screaming for it.
Is there any update about the struct inlining issues of the .NET JIT compiler?
For those who do not know it: methods with struct parameters are never even considered for inlining by the JIT compiler! That means that structs can be up to 10 times slower than using primitive types.
For details about this problem, see <https://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=93858>
As of now, this is by far the biggest performance problem in the .NET runtime for stuff like vector graphics and complex numbers. - Anonymous
September 04, 2006
The correct url:
https://connect.microsoft.com/VisualStudio/feedback/ViewFeedback.aspx?FeedbackID=93858 - Anonymous
September 05, 2006
1,3 - memory allocation (no need for class overhead)
2,4 - access performance
5 - quad has no access to vertices it is indexing
6 - to enforce reference integrity (quads should point to existing vertices, any change in vertex order should reindex quads etc)
7 - I always prefer to call AddRange method instead of adding items one by one
8 - probably MeshSection methods could have synchronization inside, but it's preferable to trade memory for performance and achieve isolation thru data copying.
9 - should be useful enough
10 - any rule could be broken when there is a good reason - Anonymous
September 05, 2006
10 - You broke no FxCop rules with the above code; however, you did break the value types should be 16 bytes or less contained in the Guidelines. Whether this applies to internal code or not I suppose Krzysztof could answer. - Anonymous
September 07, 2006
- Point3d is only used as a member field of another type. It could be thought of as a convenient grouping of fields.
2. There is no reason to protect the fields as they are nicely shielded by the API. Why give the JITter extra work?
3. Vertex is used mainly as the element type of an array. It is much more efficient to allocate one block for all vertices than allocate each element separately.
4. There is no reason to protect the fields as they are nicely shielded by the API. Why give the JITter extra work?
5. Because quads are always used by a parent object in conjunction with other objects. A quad in itself doesn't contain enough information to do anything useful.
6. Because it is an "end user" type that has some invariants to maintain. It couldn't do that if the members could be changed at will.
7. Because passing an array is essentially passing by reference. You're doing all the copying anyway, so you might as well save on function calls. Also, resizing arrays is much more efficient if you know the final size beforehand.
8. This question wouldn't be here if it were an oversight. Rule of synchronization #1: only provide synchronization logic at a level where there is enough information to do it sensibly.
9. Useful, sure. Beneficial, not so much - as in slow, although that depends on the JITter. Foreach copies the entire value type into the loop variable. Using indexed access works from the address and doesn't copy. Then again, you can have ref (not reference) types as local variables, so you could implement foreach to use them.
10 Let's see:
#1 Value types should be less than 16 bytes.
#2 Value types should be immutable.
#3 Naming: 2 letter abbreviations (3d) should be upper case.
#4 Naming: Public members should be Pascal cased.
On a very much related note, I'd be interested to know your thoughts on 'const' modifiers, particularly from the point of view of issues the WPF team had to deal with: "freezable" patterns, and the like.
Anonymous
September 07, 2006
In my last quiz I asked a few questions about a few hypothetical classes that might appear in a value-right...Anonymous
September 07, 2006
Microsoft's Rico Mariani has two very interesting posts that shed some light on when to prefer structsAnonymous
September 11, 2006
Rico has been posting performance quizzes for a couple of years now. These are always interesting and...Anonymous
September 14, 2006
I'm sure that it's the same for you: you go away on vacation, have a great time, come back and your email...Anonymous
January 23, 2007
In my last quiz I asked a few questions about a few hypothetical classes that might appear in a value-richAnonymous
February 01, 2007
My blog reader burped recently, and gave forth a post (and reply ) than Rico wrote last September, butAnonymous
May 19, 2007
Why so many people like property much than public field even though inside the property there is no other