views:

52

answers:

2

I got a rather big class library that contains a lot of code.

I am looking at how to optimize the performance of some of the code, and for some rather simple utility methods I've found that the parameter validation occupies a rather large portion of the runtime for some core methods.

Let me give a typical example:

  1. A.MethodA1 runs a loop, iterating over a collection, calling B.MethodB1 for each element
  2. B.MethodB1 processes the element and returns the result, it's a rather basic calculation, but since it is used many places, it has been put into its own method instead of being copied and pasted where needed
  3. A.MethodA1 calls C.MethodC1 with the results of B.MethodB1, and puts the result into a list that is returned at the end of the loop

In the case I've found now, B.MethodB1 does rudimentary parameter validation. Since the method calls other internal methods, I'd like to avoid having NullReferenceExceptions several layers deep into the code, and rather fail early, hence B.MethodB1 validates the parameters, like checking for null and some basic range checks on another parameter.

However, in this particular call scenario, it is impossible (due to other program logic) for these parameters to ever have the wrong values. If they had, from the program standpoint, B.MethodB1 would never be called at all for those values, A.MethodA1 would fail before the call to B.MethodB1.

So I was considering removing the parameter validation in B.MethodB1, since it occupies roughly 65% of the method runtime (and this is part of some heavily used code.)

However, B.MethodB1 is a public method, and can thus be called from the program, in which case I want the parameter validation.

So how would you solve this dilemma?

  1. Keep the parameter validation, and take the performance hit
  2. Remove the parameter validation, and have potentially fail-late problems in the method
  3. Split the method into two, one internal that doesn't have parameter validation, called by the "safe" path, and one public that has the parameter validation + a call to the internal version.

The latter one would give me the benefits of having no parameter validation, while still exposing a public entrypoint which does have parameter validation, but for some reason it doesn't sit right with me.

Opinions?

+1  A: 

I would go with option 3. I tend to use assertions for private and internal methods and do all the validation in public methods.

By the way, is the performance hit really that big?

amrinder
Yes, it really is that big for this core method. The checks are "array against null", "type against null", "index against array.length" and "array element type (comes as Array object for historical reasons) against type".
Lasse V. Karlsen
Ah ok. I would have an internal safe method which assumes all data is valid. Then a public method validates the data and then just invokes this internal method. And in all other parts of the code where you know the data is valid, you can just call the internal safe method.
amrinder
And just to note this, I have done due diligence regarding profiling :)
Lasse V. Karlsen
+1  A: 

That's an interesting question.

Hmmm, makes me think ... "code contracts" .. It would seem like it might be technically possible to statically (at compile time) have certain code contracts be proven to be fulfilled. If this were the case and you had such a compilation validation option you could state these contracts without ever having to validate the conditions at runtime.

It would require that the client code itself be validated against the code contacts.

And, of course it would inevitably be highly dependent on the type of conditions you'd want to write, and it would probably only be feasible to prove these contracts to a certain point (how far up the possible call graph would you go?). Beyond this point the validator might have to beg off, and insist that you place a runtime check (or maybe a validation warning suppression?).

All just idle speculation. Does make me wonder a bit more about C# 4.0 code contracts. I wonder if these have support for static analysis. Have you checked them out? I've been meaning to, but learning F# is having to take priority at the moment!

Update:

Having read up a little on it, it appears that C# 4.0 does indeed have a 'static checker' as well as a binary rewriter (which takes care of altering the output binary so that pre and post condition checks are in the appropriate location)

What's not clear from my extremely quick read, is whether you can opt out of the binary rewriting - what I'm thinking here is that what you'd really be looking for is to use the code contracts, have the metadata (or code) for the contracts maintained within the various assemblies but use only the static checker for at least a selected subset of contracts, so that you in theory get proven safety without any runtime hit.

Here's a link to an article on the code contracts

Phil
I'm definitely going to read up on code contracts, but for now I'm in .NET 3.5, so it's unfortunately not really an option for my current class library.
Lasse V. Karlsen
I also think this is an interesting question, regardless of platform, so the edit that added the language-agnostic tag was spot on.
Lasse V. Karlsen