All fields are treated as arrays / (engine bug ?)
by Ulf Maagaard · in Torque Game Engine · 04/12/2005 (5:24 am) · 17 replies
In file simbase.cc, method SimObject::getDataField, there is a particular part checking if the requested field is an array or not:
if(!array) {
if (const char* val = mFieldDictionary->getFieldValue(slotName))
return val;
}
This part cannot be hit, as array is never false.
Result: All fields are treated as arrays. In other words, %this.myField is just as "expensive" as %this.myArray[0]
Is this intentional? And if not, is the performance penalty of treating all fields as arrays substantial?
Thanks in advance
if(!array) {
if (const char* val = mFieldDictionary->getFieldValue(slotName))
return val;
}
This part cannot be hit, as array is never false.
Result: All fields are treated as arrays. In other words, %this.myField is just as "expensive" as %this.myArray[0]
Is this intentional? And if not, is the performance penalty of treating all fields as arrays substantial?
Thanks in advance
#2
That means that there is no reason for treating fields as arrays ever. Nevertheless this is exactly what is happening in getDataField.
However, the "unhit" section shown above actually has potential problems in it too: The call to getFieldValue does a POINTER comparison on slotName with all other pointers in a hash table. Not a string comparison.
This means that the following code, called from c++, will never work:
StringTableEntry entry = "sixthSenseRange";
const char *str = dic->getFieldValue(entry);
Even though a field called "sixthSenseRange" actually exists. Reason: The entry pointer fails the pointer comparison in getFieldValue.
Hmm...
04/12/2005 (6:23 am)
Stephen, this means that at the time of field lookup, there is no arrays. There is only strings.That means that there is no reason for treating fields as arrays ever. Nevertheless this is exactly what is happening in getDataField.
However, the "unhit" section shown above actually has potential problems in it too: The call to getFieldValue does a POINTER comparison on slotName with all other pointers in a hash table. Not a string comparison.
This means that the following code, called from c++, will never work:
StringTableEntry entry = "sixthSenseRange";
const char *str = dic->getFieldValue(entry);
Even though a field called "sixthSenseRange" actually exists. Reason: The entry pointer fails the pointer comparison in getFieldValue.
Hmm...
#3
(Note: The comments imply that my idea about arrays is not valid, so I am retracting that comment as speaking about something I don't know much about!)
04/12/2005 (6:36 am)
I couldn't tell you for sure, other than the functionality has worked for years--you are probably just trying to do a direct usage instead of an access method usage. getFieldValue is only used in SimBase.cc, and the comments in SimBase.h read:/// Some fields may be arrays, which is what the array parameter is for; if it's non-null, /// then it is parsed with dAtoI and used as an index into the array. If you access something /// as an array which isn't, then you get an empty string. /// /// <b>You don't need to read any further than this.</b> Right now, /// set/getDataField are called a total of 6 times through the entire /// Torque codebase. Therefore, you probably don't need to be familiar /// with the details of accessing them. You may want to look at Con::setData /// instead. Most of the time you will probably be accessing fields directly, /// or using the scripting language, which in either case means you don't /// need to do anything special.
(Note: The comments imply that my idea about arrays is not valid, so I am retracting that comment as speaking about something I don't know much about!)
#4
However, I've still got trouble accessing the dynamic fields' hashtable from C++, because of the pointer comparison. But that is another story - or thread, perhaps :-)
Thanks for your help!
04/12/2005 (7:00 am)
Surely it works - no questions there. Even if my assumptions are right, all we are talking about is a performance hit.However, I've still got trouble accessing the dynamic fields' hashtable from C++, because of the pointer comparison. But that is another story - or thread, perhaps :-)
Thanks for your help!
#5
!!!EDIT: DONT use this code, it is wrong. Use Bens code instead!!!
(Remember to declare it in simbase.h)
!!!EDIT: DONT use this code, it is wrong. Use Bens code instead!!!
Now, dynamic fields can be accessed from C++ as follows (from within a ConsoleMethod function):
04/12/2005 (7:32 am)
I think I got the lookup solved. In case someone else should ever want to access dynamic fields from C++, adding the following function to simbase.cc should make this possible:!!!EDIT: DONT use this code, it is wrong. Use Bens code instead!!!
const char *SimFieldDictionary::findFieldValue(StringTableEntry slotName)
{
StringTableEntry hashEntry = StringTable->lookup(slotName);
U32 bucket = HashPointer(hashEntry) % HashTableSize;
for(Entry *walk = mHashTable[bucket];walk;walk = walk->next)
if(dStricmp(walk->slotName, slotName) == 0) // 0 = IDENTICAL
return walk->value;
return NULL;
}(Remember to declare it in simbase.h)
!!!EDIT: DONT use this code, it is wrong. Use Bens code instead!!!
Now, dynamic fields can be accessed from C++ as follows (from within a ConsoleMethod function):
GameBaseData *gb = object->getDataBlock();
SimFieldDictionary *dic = gb->getFieldDictionary();
const char *str = dic->findFieldValue("sixthSenseRange");
#6
Er... Did you change it from a direct pointer comparison to a string compare, by any chance? :)
StringTableEntrys are used to indicate pointers which have been passed through StringTable->insert(). Doing this causes all instances of the same string value to point to the same actual instance in memory, allowing direct comparisons to be made (much much faster!). It also means that one doesn't have to clean up one's string memory (very handy), as the StringTable takes care of it. The proper solution in this case would be to keep the old implementation and change your access pattern to:
04/12/2005 (11:24 pm)
The array stuff is very, very low cost - especially as script almost NEVER even shows up on the profiler. I've only ever seen it in pathological cases (like extensive script processing being done every frame - not every tick, every frame).Er... Did you change it from a direct pointer comparison to a string compare, by any chance? :)
StringTableEntrys are used to indicate pointers which have been passed through StringTable->insert(). Doing this causes all instances of the same string value to point to the same actual instance in memory, allowing direct comparisons to be made (much much faster!). It also means that one doesn't have to clean up one's string memory (very handy), as the StringTable takes care of it. The proper solution in this case would be to keep the old implementation and change your access pattern to:
GameBaseData *gb = object->getDataBlock();
SimFieldDictionary *dic = gb->getFieldDictionary();
const char *str = dic->findFieldValue(StringTable->insert("sixthSenseRange"));
#7
No I didn't change pointer comparison, and yes, your solution would equally work. In fact, our solutions are doing the exact same, except that you use a Stringtable->insert and I use a Stringtable->lookup.
However, I regard a Stringtable->insert as more error-prone than a Stringtable->lookup, which is the reason why I've made findFieldValue. In cases where I strictly do not want to insert anything, I'd rather get an exception than carry on with wrong data.
04/13/2005 (10:53 am)
Thanks for you post. I now agree on the array stuff not being a problem.No I didn't change pointer comparison, and yes, your solution would equally work. In fact, our solutions are doing the exact same, except that you use a Stringtable->insert and I use a Stringtable->lookup.
However, I regard a Stringtable->insert as more error-prone than a Stringtable->lookup, which is the reason why I've made findFieldValue. In cases where I strictly do not want to insert anything, I'd rather get an exception than carry on with wrong data.
#8
I'm a little unclear what distinction you're making here... How does using lookup() change the behavior of the system at all? insert() is practically zero cost. Why are you doing a string comparison in any case, if you're passing things through StringTable->lookup? If it's not found it will return NULL, which won't match anything in the dictionary anyway. It seems like findFieldValue is a slower, less flexible version of getFieldValue, with no added opportunity for error reporting...
04/13/2005 (2:44 pm)
const char *SimFieldDictionary::getFieldValue(StringTableEntry slotName)
{
U32 bucket = HashPointer(slotName) % HashTableSize;
for(Entry *walk = mHashTable[bucket];walk;walk = walk->next)
if(walk->slotName == slotName)
return walk->value;
return NULL;
}I'm a little unclear what distinction you're making here... How does using lookup() change the behavior of the system at all? insert() is practically zero cost. Why are you doing a string comparison in any case, if you're passing things through StringTable->lookup? If it's not found it will return NULL, which won't match anything in the dictionary anyway. It seems like findFieldValue is a slower, less flexible version of getFieldValue, with no added opportunity for error reporting...
#9
1. There is NO system change, NOTHING.
2. getFieldValue and findFieldValue ARE EQUAL except from the first line.
3. The first line of findFieldValue does THE SAME as what you do in your calling convention in your first post (I take it that you mean getFieldValue instead of findFieldValue btw). EXCEPT:
4. You do a stringtable->insert, and I do a stringtable->lookup in findFieldValue.
5. Stringtable->insert(x) will insert x if it does not already exist. I do NOT want to insert anything in my example, only lookup. Thats what makes it error-prone.
Now, sticking to these points, tell me exactly what I have misunderstood, and explain why you think findFiendValue("sixthSenseRange") is slower than getFieldValue(Stringtable->insert("sixthSenseRange")).
04/14/2005 (12:29 am)
Ben, it sounds like we are somehow not understanding each other. Let me first make my understanding crystal clear (caps indicate keypoints, not shouting):1. There is NO system change, NOTHING.
2. getFieldValue and findFieldValue ARE EQUAL except from the first line.
3. The first line of findFieldValue does THE SAME as what you do in your calling convention in your first post (I take it that you mean getFieldValue instead of findFieldValue btw). EXCEPT:
4. You do a stringtable->insert, and I do a stringtable->lookup in findFieldValue.
5. Stringtable->insert(x) will insert x if it does not already exist. I do NOT want to insert anything in my example, only lookup. Thats what makes it error-prone.
Now, sticking to these points, tell me exactly what I have misunderstood, and explain why you think findFiendValue("sixthSenseRange") is slower than getFieldValue(Stringtable->insert("sixthSenseRange")).
#10
How is adding something to the string table an error condition?
04/14/2005 (4:41 pm)
Because findFieldValue does a string compare, wherea getFieldValue does a pointer compare.How is adding something to the string table an error condition?
#12
Ben, findFieldValue("sixthSenseRange") and getFieldValue(Stringtable->insert("sixthSenseRange")) both do string compare. You do it before the call to getFieldValue, I do it within findFieldValue. Look at the code, its obviously the same - no speed difference could possibly exist.
04/14/2005 (4:52 pm)
Quote:Because findFieldValue does a string compare, wherea getFieldValue does a pointer compare.
Ben, findFieldValue("sixthSenseRange") and getFieldValue(Stringtable->insert("sixthSenseRange")) both do string compare. You do it before the call to getFieldValue, I do it within findFieldValue. Look at the code, its obviously the same - no speed difference could possibly exist.
#13
04/14/2005 (4:56 pm)
Maybe this confusion stems from me giving a "wrong" type in findFieldValue. Instead of findFieldValue(StringTableEntry slotName), it should be findFieldValue(char *str). This should emphasize that the lookup goes WITHIN the call, not BEFORE the call.
#14
04/14/2005 (4:58 pm)
And just to be even more clear: Stringtable->lookup and Stringtable->insert BOTH do string compare.
#15
04/14/2005 (9:35 pm)
Please check line 6 of your code snippet, line 6 of mine - the comparison that is done against all the fields. Compare and contrast! :)
#16
I thought I had copy/pasted getFieldValue into findFieldValue, but clear not.
THANKS for pointing that out, and sorry sorry sorry. Me dumb. You thanks.
04/15/2005 (1:16 am)
Jesus, I was so sure you were talking about the stringtable stuff, that I didn't even see that.I thought I had copy/pasted getFieldValue into findFieldValue, but clear not.
THANKS for pointing that out, and sorry sorry sorry. Me dumb. You thanks.
#17
04/15/2005 (2:09 am)
Heheheh... No worries! Glad I was able to help you find a typo sort of thing. :)
Torque 3D Owner Stephen Zepp
This technique is actually somewhat common in scripting languages in general, since it provides most of the functionality for arrays without having to create and dispatch events to an actual array implementation like you are used to in other languages.