Dimitrie O. Paun wrote:

On November 26, 2003 11:22 pm, Dmitry Timoshkov wrote:


(GetVersion() & 0x80000000)==0) construct is very inefficient. It forces
compiler generate both a bit mask test and a comparison operation. While
many modern processors have very effective bit test instructions, and
we have to tell to the compiler that we want only a bit test.

Simple !(GetVersion() & 0x80000000) is much better IMO.



Do you honestly think that a compiler can't figure out this is the
same thing?!? Even if it didn't, you'd be hard pressed to even measure
such minute differences, so no, it's not very inefficient by any means.


...

So while I prefer your version for stylistic reasons, I can't agree
with the spirit of the complaint.


Something that does affect efficiency, which I debated with myself for some time, is the order of the tests. As the "if" is, by far, much more likely to be false than true, we should order the condition using a cost function that takes into account how much each condition costs to test, and how likely it is to fail (as that's the case wer'e trying to optimize - we don't really care how long it's going to take in case the "if" is true).

My original gut feeling was to place the conditions in reverse order. That is, first test for version, only then test whether there are any wildcards in the string. The reason I submitted it as I did was that the first attempt would not work. It appears that "DOSFS_GetFullName" is called before the structs needed for "GetVersion" to function are initialized. As such, reversing the order was necessary to make it work (that, or insert complicated changes throughout the entire kernel initialization).

In retrospect, this form is better. GetVersion is not a simple function, and is far less likely to fail than strchr. I guess ideal would be to have a global inside kernel.dll that tells us whether wer'e NT or not. There is no such global at the moment, however, and I did not wish to introduce one.

--
Shachar Shemesh
Open Source integration & consulting
Home page & resume - http://www.shemesh.biz/





Reply via email to