@pspeed said:
Can you help point me to the JME code that does this? Thanks.
Well, context just got changed a bit with latest commits, but the real difference is in DefineList.equalsParams. This method is called very often. It compares if parameters given in given Material match the ones for which the shader was compiled. In many cases it will be true. Still, when comparing int, float and boolean values, it takes the primitive value from material define and does
[java]
String newValue = val.toString();
String current = defines.get(key);
if (!newValue.equals(current)) {
return false;
}
[/java]
Which converts every float/int/boolean in material list for each field to string each frame. It is not only garbage, but actually non-trivial formatting code in case of floats.
I have replaced it with bit ugly code
[java]
Object current = defines.get(key);
if ( current == null ) {
return false;
}
if (current instanceof Number && val instanceof Number ) {
if ( ((Number)current).doubleValue() != ((Number)val).doubleValue() ) {
return false;
}
} else if (!val.toString().equals(current.toString())) {
return false;
}
[/java]
Which avoid the conversion to string for numbers (and similar code for booleans). It still has a string comparison code there, to work with serialized objects, which might have strings somewhere.
This required changing contents of defines to have primitives instead of strings, so there is an extra cost on âgetâ call - previously it was just getting already prepared string from map, here it has to convert it on the fly. Still, get is called only when something has changed, while equalParameters is called a lot more often, so it was a reasonable tradeoff, at least in my tests.
Edit: Now when I look at it, get might be not called at all ever, so even better.
The part I donât understand is why convert to String at all instead of just comparing the values. There is something Iâm missing by having not read through the code recently. Presumably the types would be consistent and a simple value1.equals(value2) would be a lot easier than the split out code.
I have done this stuff to stay compatible with serialized objects. They currently have strings put inside values. All newly created define lists would have proper types and .equals would be good enough, but code would crash for any older .j3mo loaded from disk. At least I think so, Iâm not using .j3mo myselfâŚ
@abies said:
I have done this stuff to stay compatible with serialized objects. They currently have strings put inside values. All _newly_ created define lists would have proper types and .equals would be good enough, but code would crash for any older .j3mo loaded from disk. At least I think so, I'm not using .j3mo myself...
I didnât even know there was a j3mo file. I wonder what theyâre for and who would scream if we broke it.
The other alternative is since defines are typeless then never store them as real objects at all. Instead change them to string when they are set and just keep them as strings. Then you can still do direct value comparisons because you will know that everything is a string. If you intern() them then the compare is the same performance as something like a Float or Double compare even. (Presuming the values of defines are not varied enough to blow up permgen.)
@pspeed said:
The other alternative is since defines are typeless then never store them as real objects at all. Instead change them to string when they are set and just keep them as strings. Then you can still do direct value comparisons because you will know that everything is a string. If you intern() them then the compare is the same performance as something like a Float or Double compare even. (Presuming the values of defines are not varied enough to blow up permgen.)
But problem is that we are not comparing define lists with each other, we are comparing them against material parameters, which are primitive types (and probably have to be to fit the api for setting uniforms).
@abies said:
But problem is that we are not comparing define lists with each other, we are comparing them against material parameters, which are primitive types (and probably have to be to fit the api for setting uniforms).
Ah, yes, the other issue.
Itâs been too long since I went through this code but I remember not liking something about all of that. A particular define for a technique only needs to change when its linked parameter does⌠and that could be cached as a string. But there was some issue with this and I forget.
I wonder what theyâre for and who would scream if we broke it.