Speeding Up Function Lookups

[Valik]

I just called a MsgBox function via:

return (this->*m_FuncList[nFunction].lpFunc)(vParams, iNumParams, vResult);

I find myself giggling in a troubling way.

I'll have to rework about 20% of the functions (single line funcs, Larry's magic number funcs , not same 3 parameters etc) but even so this is very cool.  Surely a couple of KB off the exe size.

I think I giggled in a troubling way the first time I figured out how to use this method, too. There's just something exciting about functions being looked up at run-time instead of at compile time (Especially when you have the guarantee you will actually be calling a function and not inviting a crash).

By the way, just a couple other things I forgot to mention about pointer-to-members.

The ->* notation requires parenthesis to be around the function and object to bind them because ->* is rather loose on binding. Remove the parenthesis and the compiler will complain.

// Compiler error, missing () around "this->*m_FuncList[nFunction].lpFunc"
this->*m_FuncList[nFunction].lpFunc(vParams, iNumParams, vResult);

The other notation is .* and is obviously used for real objects (or references) and not pointers. It does not have loose binding, so this is perfectly fine:

*this.*m_FuncList[nFunction].lpFunc(vParams, iNumParams, vResult);

OR

object.*m_FuncList[nFunction].lpFunc(vParams, iNumParams, vResult);

I think that's everything on them...

[Jon]

40 functions done...a million to go. Looks much neater though with just a single place needed to describe a function:

expandcollapse popup

AU3_FuncInfo AutoIt_Script::m_FuncList[] = 
{
    {"ADLIBDISABLE", F_AdlibDisable, 0, 0},
    {"ADLIBENABLE", F_AdlibEnable, 1, 2},
    {"BLOCKINPUT", F_BlockInput, 1, 1},
    {"CHR", F_Chr, 1, 1},
    {"CONTROLGETFOCUS", F_ControlGetFocus, 1, 2},
    {"DEC", F_Dec, 1, 1},
    {"DIRCOPY", F_DirCopy, 2, 3},
    {"ENVGET", F_EnvGet, 1, 1},
    {"ENVSET", F_EnvSet, 1, 2},
    {"INIREAD", F_IniRead, 4, 4},
    {"INIWRITE", F_IniWrite, 4, 4},
    {"INT", F_Int, 1, 1},
    {"MOUSEDOWN", F_MouseDown, 1, 1},
    {"MOUSEUP", F_MouseUp, 1, 1},
    {"MSGBOX", F_MsgBox, 3, 4},
    {"NUMBER", F_Number, 1, 1},
    {"PROCESSCLOSE", F_ProcessClose, 1, 1},
    {"PROCESSEXISTS", F_ProcessExists, 1, 1},
    {"PROCESSWAIT", F_ProcessWait, 1, 2},
    {"PROCESSWAITCLOSE", F_ProcessWaitClose, 1, 2},
    {"RUN", F_Run, 1, 3},
    {"RUNWAIT", F_RunWait, 1, 3},
    {"SEND", F_Send, 1, 2},
    {"SLEEP", F_Sleep, 1, 1},
    {"STRING", F_String, 1, 1},
    {"TRAYTIP", F_TrayTip, 3, 4},
    {"WINACTIVATE", F_WinActivate, 1, 2},
    {"WINACTIVE", F_WinActive, 1, 2},
    {"WINCLOSE", F_WinClose, 1, 2},
    {"WINEXISTS", F_WinExists, 1, 2},
    {"WINKILL", F_WinKill, 1, 2},
    {"WINMINIMIZEALL", F_WinMinimizeAll, 0, 0},
    {"WINMINIMIZEALLUNDO", F_WinMinimizeAllUndo, 0, 0},
    {"WINMOVE", F_WinMove, 4, 6},
    {"WINSETTITLE", F_WinSetTitle, 3, 3},
    {"WINSHOW", F_WinShow, 3, 3},
    {"WINWAIT", F_WinWait, 1, 3},
    {"WINWAITACTIVE", F_WinWaitActive, 1, 3},
    {"WINWAITCLOSE", F_WinWaitClose, 1, 3},
    {"WINWAITNOTACTIVE", F_WinWaitNotActive, 1, 3}

};

[Nutster]

That doesn't work here, David.  The compiler will complain:

The only 2 ways around that:

1) Make each function static (As Jon already mentioned)

2) Use pointer-to-members. 

That brings me to *this.  A pointer-to-member is absolutely useless by itself.  It requires an object to be bound to.  In this case, *this is a good match because it is of the right type and is an instantiated object.  g_oScript would also be a safe choice, although, that uses a global variable which we want to avoid.

Your idea would be fine were we not calling class members, but I don't see how to do it like that when we are calling them.

You are right. Because the pointers are in a static data member array (which is the only way to initialize the array with array construction ), the functions are expected to be static member functions or global functions. Phooey!

In order to use non-static member functions, the static array must be called through an object. *this is as good as any, especially if we want the member functions to use the data of the object.

Hmm, we don't need to be passing all those arguments. We can just access them directly in from the object and save some stack space!

Edited May 6, 2004 by Nutster

[Nutster]

40 functions done...a million to go.  Looks much neater though with just a single place needed to describe a function:

AU3_FuncInfo AutoIt_Script::m_FuncList[] = 
{
    {"ADLIBDISABLE", F_AdlibDisable, 0, 0},
....
    {"WINWAITNOTACTIVE", F_WinWaitNotActive, 1, 3}

};

Now we just need a binary search feature to find the wanted entry! Hmm, something like...

nPos = sizeof(m_FuncList)/sizeof(AU3_FuncInfo)-1;  // look at last entry
nDiff = nPos / 2;

while (bFuncFound == false)
{
    if (asFunction < this->*m_name)
    {
         nPos -= nDiff;
    }
    else if (asFunction > this->*m_name)
    {
         nPos += nDiff;
    }
    else // equal
    {
         switch (this ->* m_func())
         {
         case AUT_ERR:
              // abort
         case AUT_OK:
              // nice and clean.  Access vResult from *this
              bFuncFound = true;
         }
    }
    if (nDiff > 1)
        nDiff /= 2;
    else
        // I can never get this clean with out a lot of work!
}

Edited May 6, 2004 by Nutster

[Valik]

Hmm, we don't need to be passing all those arguments.  We can just access them directly in from the object and save some stack space! 

If the parser is ever moved out of AutoIt_Script (And I think it should be as it's a parser, not a script, and each class should represent only one object or something like that ), that will cause more rewriting. It would be no trouble to pass in an object of AutoIt_Script to the parser for use instead of *this, but you'd have to turn around and add back in all the parameters being passed if it were removed now and then the parser is moved later. Did that make sense? I think I said that more convulated than I need to...

[Nutster]

That does make sence. AutoIt_Script is pretty big right now and could benifit from being broken down a bit.

Let's see, we could put the keywords in their own class and the functions in their own class and the macros in their own class. Then we (Jon) would write a public interface function that would choose the correct action.

Cscript_functions::launch_function(AString name, VariantList vParams, Variant &vResult);

The nNumParams is not needed as it is just the size of vParams.

[Valik]

Now we're talking. I saw a long time ago that there was some common stuff between functions, keywords, macro's and user functions. They basically all share the same interface, and now with the new pointer-to-member method Jon is working on, the implementation for all those can be abstracted away into a common interface as well (It could before as well, it was just a little clunkier with that case structure). It should be possible to design a base class to serve as an interface which provides the basic operations common to each (Probably as virtual functions, or perhaps a pure-virtual base?).

In fact, if each class (Function, User-Function, Macro, Keyword) is derived from a base class with a common interface, then the parser/lexer can be simplified quite a bit. The parser/lexer will no longer need to know anything specific about those groups nor will there need to be 4 different functions for figuring out which is which and how to act on them all.

[Jon]

The nNumParams is not needed as it is just the size of vParams.

The reason I've tended to do this is that I ass-umed that it would give a smaller exe size rather than having every other function doing a vParams.size() call. Edited May 6, 2004 by Jon

[Jon]

80 functions done. Jebus this is taking some time...

[Jon]

Ok, need some help. I've spent about 5 hours implementing the new function calls and under VC7 it is about 8KB bigger - but it compiles without errors and run ok, so I went to compile under VC6 (as usual) to see the size difference and it won't compile at all:

Declarations before the AutoIt_Script class:

class AutoIt_Script;         // Forward declaration

typedef AUT_RESULT (AutoIt_Script::*AU3_FUNCTION)(VectorVariant &vParams, uint iNumParams, Variant &vResult);

typedef struct  
{
    //char   szName[20];     // Function name 
    AString      sName;
    AU3_FUNCTION    lpFunc;      // Pointer to function
    uchar    cMin;      // Min params
    uchar    cMax;      // Max params    
} AU3_FuncInfo;

Declaration in the class:

static AU3_FuncInfo m_FuncList[];

And the actual table in script.cpp:

AU3_FuncInfo AutoIt_Script::m_FuncList[] = 
{
    {"ADLIBDISABLE", F_AdlibDisable, 0, 0},
    {"ADLIBENABLE", F_AdlibEnable, 1, 2},
    {"ASC", F_Asc, 1, 1},
...
...

I get an error of every line of the code in script.cpp:

Z:\Code\CVSROOT\autoit\src\script.cpp(61) : error C2440: 'initializing' : cannot convert from 'char [13]' to 'AU3_FuncInfo'
        No constructor could take the source type, or constructor overload resolution was ambiguous
Z:\Code\CVSROOT\autoit\src\script.cpp(61) : error C2440: 'initializing' : cannot convert from 'int (__thiscall AutoIt_Script::*)(class VectorVariant &,unsigned int,class Variant &)' to 'AU3_FuncInfo'
        No constructor could take the source type, or constructor overload resolution was ambiguous
Z:\Code\CVSROOT\autoit\src\script.cpp(61) : error C2440: 'initializing' : cannot convert from 'const int' to 'AU3_FuncInfo'
        No constructor could take the source type, or constructor overload resolution was ambiguous
Z:\Code\CVSROOT\autoit\src\script.cpp(61) : error C2440: 'initializing' : cannot convert from 'const int' to 'AU3_FuncInfo'
        No constructor could take the source type, or constructor overload resolution was ambiguous

Help.

[Valik]

That's interesting. Not that it should actually matter, but how about using the C++ notation for declaring the struct (And if that doesn't work, try making it a class just to see what the compiler says...):

struct AU3_FuncInfo
{
//char   szName[20];     // Function name 
AString   sName;
AU3_FUNCTION lpFunc;      // Pointer to function
uchar   cMin;      // Min params
uchar   cMax;      // Max params    
};

[Jon]

Same error for both suggestions.,

[Valik]

VS .NET is supposed to be highly standards compliant and that should be standards compliant code you've been writing so I don't understand why it doesn't compile. However, surely VC6 would at least be that compliant as well. What I'm trying to say is VC7 can do some stuff VC6 can't, but that shouldn't be one of them (Especially considering that this method is very similar to how MFC implements some stuff)

Does my Cow example compile on VC6? It compiled fine for me on VC7. I'm going to try it on gcc as gcc is the most annoyingly compliant compiler I've tried to use. Results in a few minutes.

Edit: Forgot a couple sentences the first time.

Edited May 9, 2004 by Valik

[Jon]

Does my Cow example compile on VC6?  It compiled fine for me on VC7.

Works on VC7, fails with the same error under VC6.

[Valik]

gcc forced me to fully qualify the function names like this (This is using my Cow example which I copied and pasted to a file and ran through g++ 3.3.3 on Debian):

{"Betsy", &Cow::Speak}

So, see if qualifying the function helps...

[Jon]

Nah, it has a problem with all 4 initializers, not just the function one.

[Valik]

I'm out of ideas if changing the function signature doesn't make it go away. I can compile fine with both gcc and VC7.1 (The only 2 compilers I have available).

[Nutster]

Ok, need some help.  I've spent about 5 hours implementing the new function calls and under VC7 it is about 8KB bigger -  but it compiles without errors and run ok, so I went to compile under VC6 (as usual) to see the size difference and it won't compile at all.

I get an error of every line of the code in script.cpp:

Z:\Code\CVSROOT\autoit\src\script.cpp(61) : error C2440: 'initializing' : cannot convert from 'char [13]' to 'AU3_FuncInfo'
        No constructor could take the source type, or constructor overload resolution was ambiguous
Z:\Code\CVSROOT\autoit\src\script.cpp(61) : error C2440: 'initializing' : cannot convert from 'int (__thiscall AutoIt_Script::*)(class VectorVariant &,unsigned int,class Variant &)' to 'AU3_FuncInfo'
        No constructor could take the source type, or constructor overload resolution was ambiguous
Z:\Code\CVSROOT\autoit\src\script.cpp(61) : error C2440: 'initializing' : cannot convert from 'const int' to 'AU3_FuncInfo'
        No constructor could take the source type, or constructor overload resolution was ambiguous
Z:\Code\CVSROOT\autoit\src\script.cpp(61) : error C2440: 'initializing' : cannot convert from 'const int' to 'AU3_FuncInfo'
        No constructor could take the source type, or constructor overload resolution was ambiguous

Help.

It seems like it is ignoring one level of the curly braces. Do you have the latest patch level (SP 6)?

What about making a constructor in the struct that takes all the given arguments? A struct and a class are identical in implementation in C++ except that a struct uses public access by default and and a class uses private access.

I will experiment tomorrow. I have VC6 SP6 at home. Hey Valik, where can I get your Cow example?

[Valik]

David, the Cow example is in this post. Basically it's somewhere above but that link should take you to it.

With that Cow example, I tried making a constructor for the struct but gcc complained about that (I didn't try with VC7.1, though, but if gcc complains it's usually not standards conforming).

I do agree with David, though. The error message says it can't convert from each type to the type of the struct, which I find odd. It's like it is ignoring that first brace.

[Gene]

David, the Cow example is in this post. Basically it's somewhere above but that link should take you to it.

With that Cow example, I tried making a constructor for the struct but gcc complained about that (I didn't try with VC7.1, though, but if gcc complains it's usually not standards conforming).

I do agree with David, though.  The error message says it can't convert from each type to the type of the struct, which I find odd.  It's like it is ignoring that first brace.

Is there any downside to adding one or a pair, and seeing what the result is? From what you've said, this is very weird, so try something weird if it won't bite you.

Gene

PS. Don't expect frequent comments from me here!