Property Code Generation

[johnspackman]

The code generation for properties is complex and obscure, particularly for setXxx methods; the generation is spread across a number of __emitXxx methods in qx.core.Property and while each method is adding a given feature, the feature is customised (or sometimes radically different) different depending on the variant being produced…and there can be up to 5 or 6 variants like “set”, “setTheme”, “setRuntime”, “reset”, “refresh”, “init” etc. The net result is that to understand one line of generated code, you have to understand an entire code path through the length of qx.core.Property

The code is written to be mindful of performance problems in Javascript in early browsers, and this hand crafting serves to complicate the code; modern Javascript engines may not need hand-optimisation any more, and may be able to make more performant code if only it was written more “normally”.

The hand optimisation has two main impacts on the code - (1) that the methods generated must be completely inline and with minimal whitespace or other formatting, and (2) that string concatenation or manipulation must be completely avoided in favour of [].join(‘’) for code and a huge lookup table for property names (ie instead of members[“set” + propertyName] the result of ”set” + propertyName must be calculated in advance - which means maybe 10 strings per property)

The implementation of setXxx methods is getting longer - for example qx.ui.core.Widget.setDecorator is now 39 lines long when beautified, and apart from the variable the value is stored in, it is identical to setThemedDecorator. Were it not for the reliance on inlining, this overlap of code could be handled by a common function.

Because the __emitXxx methods generate code for many different variants, making a simple change can easily have unintended side effects because you're not just changing one method, you're changing 5, 6 or 7; and when the generated code is this size, it is doubly hard to debug.

Here’s setDecorator’s code:

function anonymous(value
    /**/
) {
    var equ = function(a, b) {
        return a === b;
    };
    var prop = qx.core.Property;
    if (arguments.length !== 1) prop.error(this, 1, "decorator", "set", value);
    if (value === undefined) prop.error(this, 2, "decorator", "set", value);
    if (equ.call(this, this.$$user_decorator, value)) return value;
    var msg = "Invalid incoming value for property 'decorator' of class 'qx.ui.core.Widget'";
    if (value !== null)
        if (!(value !== null && qx.theme.manager.Decoration.getInstance().isValidPropertyValue(value))) prop.error(this, 5, "decorator", "set", value);
    var computed, old;
    if (this.$$runtime_decorator !== undefined) {
        old = computed = this.$$runtime_decorator;
        this.$$user_decorator = value;
    } else if (this.$$user_decorator !== undefined) {
        old = this.$$user_decorator;
        computed = this.$$user_decorator = value;
    } else if (this.$$theme_decorator !== undefined) {
        old = this.$$theme_decorator;
        computed = this.$$user_decorator = value;
    } else if (this.$$useinit_decorator) {
        old = this.$$init_decorator;
        delete this.$$useinit_decorator;
        computed = this.$$user_decorator = value;
    } else {
        computed = this.$$user_decorator = value;
    }
    if (equ.call(this, old, computed)) return value;
    if (old === undefined) old = this.$$init_decorator;
    this._applyDecorator(computed, old, "decorator", "set");
    var reg = qx.event.Registration;
    if (reg.hasListener(this, 'changeDecorator')) {
        reg.fireEvent(this, 'changeDecorator', qx.event.type.Data, [computed, old])
    }
    return value;
}

One possible solution would be to remove the reliance on inline methods, and have a generic setPropertyValue method which is passed parameters from the generated code; keeping the generated code minimal and defining an internal API method to implement storing the property value.

If inline methods are so important for performance then switching to a templated implementation for each variant would also be a huge simplification.

WRT hand-optimised code, this SO article http://stackoverflow.com/questions/7299010/why-is-string-concatenation-faster-than-array-join says that [].join is not the fastest any more, but that seems to have changed again because that article’s benchmark test shows it’s fractionally faster on Chrome, but on the FF and IE takes 70%-80% of the time than string concatenation, and a massive 33% of the time on Safari. However, this https://jsperf.com/glue-a-string-together test says the opposite, illustrating once again that gaming the compiler has its limitations.

[derrell]

My experience is that classes are not instantiated frequently, in which case the differences in the time it takes for [].join vs. String.+ vs. various other methods isn't really an issue. (Does your experience match mine?) If we can take that as a starting point, then the choice of best implementation should be based on readability and maintainability rather than speed. Having a non-inline function would seem to be far superior in that regard, and could even allow for overriding it -- or parts of it -- easily. I like the concept of a setPropertyValue method, but I'd like to see it dispatch out to individual separate methods, probably based on those various if conditions, that could be overridden but subclasses.

[johnspackman]

re: performance differences, i tend to agree - I imagine that it's performant enough that I've never worried about using [].join, and the more modern the browser the less there is to worry about.

If we reduced the amount of generated code then we're reducing the amount of concatenation too, and staying out of the debate altogether.

I think my personal favourite at the moment is the minimal code generation plussetPropertyValue, but Im not sure it's compilicated enough to need separate functions - although there are lots of __emitXxx methods and the 6-7 different variants, I suspect that once you reduce it to a normal bit of code setPropertyValue is just a simple function.

[patrick-fischer-hirschstein]

Maybe it is a good first step to look at the qooxdoo next implementation of the property handling. I was following that many months ago. I remember 1&1 changed that part completely.

[johnspackman]

@stahl-riesa good point; qooxdoo next supports a slightly different set of features, but it uses a single closure method and no code generation at all https://github.com/qooxdoo/next/blob/master/framework/source/class/qx/Class.js#L506-L591