[pH4Lk0n]

Hi,

Thank's for update.
I've checked my test cases and I see huge performance improvement.
Now I don't see much of a difference between test with and without whenRules.

Tests in real life example also doesn't show any performance problems. Even though I've also increased use of when rules.

I will perform more test's but for now it looks like problem is resolved.
Thank you very much.
Best regards

Mariusz Goch

[Isomorphic]

We've now made these optimizations in the framework source and in our testing they resolve the slowness in the test case you shared.
Please try the next nightly SGWT build - 13.0 or 13.1 branch, dated Sep 20 or above and let us know if this continues to be a problem for you

Thanks
Isomorphic Software

[Isomorphic]

Hi pH4Lk0n
A quick update on this issue:
The bulk of the slowness here is GWT specific. The performance problems go away with equivalent code written in pure SmartClient/JS.
We are still investigating this (and pinning down / resolving the cause of the slowness in GWT may still take some time).

However there are some changes we can make in the SmartClient framework code which should have a huge impact on this issue.
These are in progress and we'll be following up here when they're in place

Regards
Isomorphic software

[pH4Lk0n]

Hi,
Any news on this problem?

Best regards
Mariusz Goch

[Isomorphic]

Wow! Nice job figuring this out. We were already working on further eliminating the recursive calls, but this autoComplete:"native" thing is definitely a separate problem, and we'll check it out.

[pH4Lk0n]

Ok. You've got me real thinking.
We are talking about this problem - recursive calls of processRules in particular scenario.
https://forums.smartclient.com/forum...539#post270539

It was before tested on SmartGWT 13.0p 2023-07-10.
To be sure I've updated to latest nightly: SmartGWT 13.0p 2023-07-15 and I still see this problem.
But If you don't see the problem I looked further and I'm shocked.
Found it in my skin modifications. When switch to basic Tahoe problem does not exists.
So I compared all settings and found the problem one:

isc.TextItem.addProperties({
    autoComplete: "native",
    width: formItemWidth,
    height: formItemHeight,
    showFocused: true,
    textBoxStyle: "textItemLite"
});

When you set autoComplete to "native" the bug with recursive calls appears without it it's gone.

And testing some more it's also connected to poor results in other cases.

So as I see we've got to problems:
- so slow processRule with autoComplete = native
- so many calls of processRules

So testing some more in my real life example (all tests done with SmartGWT 13.0p 2023-07-15) -of many calculations and multiple rules on a form.
1. With autoComplete = native: around 250ms
2. Without autoComplete native: 40ms
3. Adding patch to minimize calls of processRules: 10-15ms

So the problem with autoComplete is not only connected to recursive calls. In above example I also checked and there are no recursive calls.
But this change significantly affects processRules single call which I observed in profiles. From around 25-40ms for one call it dropped to 2ms.

Hope this help solving both problems.

Best regards
Mariusz Goch

[Isomorphic]

Thank you for your work on this test case. Regarding the typecast lines, we are not reproducing a performance difference, or really any performance problem, with your test case.

Is it possible this was not tested with a build that has the first round of optimizations?

Or was your intent here not to reproduce a performance problem per se, but just show what appear to be redundant processRules calls?

[pH4Lk0n]

Hi,

3. About this suggestion again.
I'm testing with my real life example which have a handler with multiple calculation and field updates with setValue().

Without whenRules it's working just fine and takes around 15-20ms to run - this is without FormIf functions that was replaces with whenRules.

With whenRules enabled it takes more than 500ms. It's after fix with (String)null so no recursive processRules calls but still it's called 54 times and takes a lot of time. On testing machines it takes a lot more.

I've created a simple proof of concept.

isc.RulesEngine.addProperties({
    processRules : function (rules, component) {
        console.log("processRules", component.ID, rules);
        if (!this.processRulesTimeout) {
            this.processRulesTimeout = {};
        }
        if (this.processRulesTimeout[component.ID]) {
            clearInterval(this.processRulesTimeout[component.ID]);
        }
        this.processRulesTimeout[component.ID] = setTimeout(() => {
            this.processRules2(rules, component);
        }, 0);
    }
});

And in processRules2 method is original code of processRules.
Now same scenario runs in about 20-25ms and original processRules are called only 2 times.

I now this code is plain simple but it's just a proof of concept and maybe there are some caveat's that I'm not aware of but considering huge performance boost I think it's worth looking at.

Best regards
Mariusz Goch

[pH4Lk0n]

Hi,
1. I've isolated test case.

public void testRules() {
        boolean whenRules = true;

        DataSource ds = getRulesDataSource();

        HLayout layout = new HLayout(8);

        ListGrid listGrid = new ListGrid();
        listGrid.setDataSource(ds);
        listGrid.setWidth(300);
        listGrid.setHeight(300);

        listGrid.fetchData();

        layout.addMember(listGrid);

        ValuesManager vm = new ValuesManager();
        vm.setDataSource(ds);

        DynamicForm form1 = new DynamicForm();
        BoxFormat.form(form1);
        form1.setDataSource(ds);
        form1.setValuesManager(vm);

        ArrayList<FormItem> items1 = onForm1(whenRules);

        if (whenRules) {
            setReadOnly(items1);
        }

        form1.setFields(items1.toArray(new FormItem[items1.size()]));

        layout.addMember(form1);

        DynamicForm form2 = new DynamicForm();
        BoxFormat.form(form2);
        form2.setDataSource(ds);
        form2.setValuesManager(vm);
        form2.setNumCols(4);
        form2.setColWidths("30%", "25%", "20%", "25%");

        ArrayList<FormItem> items2 = onForm2(whenRules);
        if (whenRules) {
            setReadOnly(items2);
        }

        form2.setFields(items2.toArray(new FormItem[items2.size()]));

        layout.addMember(form2);

        listGrid.addRecordClickHandler(event -> {
            long time = (new Date()).getTime();
            vm.editRecord(event.getRecord());
            long time2 = (new Date()).getTime();
            console.log("editRecord time: ", time2 - time);
        });

        this.addChild(layout);
    }

    public DataSource getRulesDataSource() {
        DataSource ds = new DataSource();
        ds.setClientOnly(true);

        DataSourceIntegerField fieldId = new DataSourceIntegerField("id");
        fieldId.setPrimaryKey(true);
        ds.addField(fieldId);

        DataSourceTextField fieldType = new DataSourceTextField("type");
        fieldType.setValueMap("ware", "service", "period");
        ds.addField(fieldType);

        DataSourceTextField fieldPeriod = new DataSourceTextField("period");
        fieldPeriod.setValueMap("day", "week", "2week", "month", "2month");
        ds.addField(fieldPeriod);

        DataSourceTextField fieldStatus = new DataSourceTextField("status");
        fieldStatus.setValueMap("new", "progress", "finished", "canceled");
        ds.addField(fieldStatus);

        DataSourceTextField fieldVat = new DataSourceTextField("vat");
        ds.addField(fieldVat);

        DataSourceFloatField fieldCost = new DataSourceFloatField("cost");
        fieldCost.setFormat("0.00");
        ds.addField(fieldCost);

        DataSourceFloatField fieldPriceMargin = new DataSourceFloatField("price_margin");
        fieldPriceMargin.setDecimalPad(2);
        ds.addField(fieldPriceMargin);

        DataSourceFloatField fieldPriceBaseNet = new DataSourceFloatField("price_base_net");
        fieldPriceBaseNet.setFormat("0.00");
        ds.addField(fieldPriceBaseNet);

        DataSourceFloatField fieldPriceBase = new DataSourceFloatField("price_base");
        fieldPriceBase.setFormat("0.00");
        ds.addField(fieldPriceBase);

        DataSourceFloatField fieldDiscount = new DataSourceFloatField("discount");
        fieldDiscount.setDecimalPad(2);
        ds.addField(fieldDiscount);

        DataSourceFloatField fieldPriceNet = new DataSourceFloatField("price_net");
        fieldPriceNet.setFormat("0.00");
        fieldPriceNet.setRequired(true);
        ds.addField(fieldPriceNet);

        DataSourceFloatField fieldPrice = new DataSourceFloatField("price");
        fieldPrice.setFormat("0.00");
        fieldPrice.setRequired(true);
        ds.addField(fieldPrice);

        DataSourceFloatField fieldPriceProfit = new DataSourceFloatField("price_profit");
        fieldPriceProfit.setFormat("0.00");
        ds.addField(fieldPriceProfit);

        Record r1 = new Record();
        r1.setAttribute("id", 1);
        r1.setAttribute("product_id", 1);
        r1.setAttribute("type", "period");
        r1.setAttribute("status", "new");
        r1.setAttribute("quantity", "11");
        r1.setAttribute("total_net", "11.12");
        r1.setAttribute("total", "11.12");
        r1.setAttribute("total_profit", "11.12");
        r1.setAttribute("vat", "23");
        r1.setAttribute("cost", "11.12");
        r1.setAttribute("price_margin", "11.12");
        r1.setAttribute("price_base_net", "11.12");
        r1.setAttribute("price_base", "11.12");
        r1.setAttribute("showDiscount", true);
        r1.setAttribute("discount", "11.12");
        r1.setAttribute("price_net", "11.12");
        r1.setAttribute("price", "11.12");
        r1.setAttribute("discount", "11.12");


        Record r2 = new Record();
        r2.setAttribute("id", 2);
        r2.setAttribute("type", "ware");
        r2.setAttribute("status", "progress");
        r2.setAttribute("quantity", "11");
        r2.setAttribute("total_net", "0.99");
        r2.setAttribute("total", "0.99");
        r2.setAttribute("total_profit", "0.99");
        r2.setAttribute("vat", "23");
        r2.setAttribute("cost", "0.99");
        r2.setAttribute("price_margin", "0.99");
        r2.setAttribute("price_base_net", "0.99");
        r2.setAttribute("price_base", "0.99");
        r2.setAttribute("showDiscount", true);
        r2.setAttribute("discount", "0.99");
        r2.setAttribute("price_net", "0.99");
        r2.setAttribute("price", "0.99");
        r2.setAttribute("discount", "0.99");

        Record r3 = new Record();
        r3.setAttribute("id", 3);
        r3.setAttribute("product_id", 2);
        r3.setAttribute("type", "period");
        r3.setAttribute("showDiscount", false);
        r3.setAttribute("status", "progress");

        ds.setTestData(r1, r2, r3);

        return ds;
    }

    public ArrayList<FormItem> onForm1(boolean whenRules) {
        ArrayList<FormItem> items = new ArrayList<FormItem>();

        items.add(new TextItem("id"));

        AdvancedCriteria criteriaProduct = new AdvancedCriteria("product_id", OperatorId.NOT_NULL);

        TextItem itemProductId = new TextItem("product_id");
        if (whenRules) {
            itemProductId.setVisibleWhen(criteriaProduct);
        }
        items.add(itemProductId);

        TextItem itemName = new TextItem("name");
        items.add(itemName);

        TextItem itemCode = new TextItem("code");
        items.add(itemCode);

        RadioGroupItem itemType = new RadioGroupItem("type");
        itemType.setDefaultValue("ware");
        itemType.setVertical(false);
        items.add(itemType);

        SelectItem itemPeriod = new SelectItem("period");
        itemPeriod.setDefaultValue("month");
        if (whenRules) {
            itemPeriod.setVisibleWhen(new AdvancedCriteria("type", OperatorId.EQUALS, "period"));
        }
        items.add(itemPeriod);

        SpinnerItem itemPeriodQuantity = new SpinnerItem("period_quantity");
        itemPeriodQuantity.setDefaultValue(1);
        itemPeriodQuantity.setMin(1);
        itemPeriodQuantity.setStep(1);
        itemPeriodQuantity.setWriteStackedIcons(true);
        if (whenRules) {
            itemPeriodQuantity.setVisibleWhen(new AdvancedCriteria("type", OperatorId.EQUALS, "period"));
        }
        items.add(itemPeriodQuantity);

        DateItem itemDateStart = new DateItem("date_start");
        if (whenRules) {
            itemDateStart.setVisibleWhen(new AdvancedCriteria("type", OperatorId.EQUALS, "period"));
        }
        itemDateStart.setDefaultValue(new Date());
        items.add(itemDateStart);

        DateItem itemDateEnd = new DateItem("date_end");
        if (whenRules) {
            itemDateEnd.setVisibleWhen(new AdvancedCriteria("type", OperatorId.EQUALS, "period"));
            itemDateEnd.setReadOnlyWhen(new AdvancedCriteria("period", OperatorId.NOT_EQUAL, "other"));
        }
        items.add(itemDateEnd);

        TextItem itemPKWiU = new TextItem("pkwiu");
        itemPKWiU.setWidth(120);
        items.add(itemPKWiU);

        TextItem itemSerial = new TextItem("serial");
        items.add(itemSerial);

        items.add(new SelectItem("status"));

        return items;
    }

    public ArrayList<FormItem> onForm2(boolean whenRules) {
        ArrayList<FormItem> items = new ArrayList<FormItem>();

        BooleanItem itemShowDiscount = new BooleanItem("showDiscount");
        itemShowDiscount.setDefaultValue(false);
        itemShowDiscount.setStartRow(false);
        itemShowDiscount.setEndRow(true);
        itemShowDiscount.setShowTitle(false);
        itemShowDiscount.setColSpan(2);
        itemShowDiscount.setAlign(Alignment.CENTER);
        itemShowDiscount.addChangedHandler(event -> {
            DynamicForm form = event.getForm();

            form.setValue("price_base_net", "");
            form.setValue("price_base", "");
            form.setValue("discount", "");
        });

        TextItem itemCost = new TextItem("cost");
        itemCost.setDefaultValue(0);
        itemCost.setEndRow(true);
        itemCost.setWidth("*");
        items.add(itemCost);

        TextItem itemPriceMargin = new TextItem("price_margin");
        itemPriceMargin.setWidth(70);
        itemPriceMargin.setHint("%");
        itemPriceMargin.setEndRow(true);
        items.add(itemPriceMargin);

        SelectItem itemVat = new SelectItem("vat");
        itemVat.setDefaultValue("23");
        itemVat.setWidth(70);
        items.add(itemVat);

        items.add(itemShowDiscount);

        TextItem itemPriceBaseNet = new TextItem("price_base_net");
        itemPriceBaseNet.setWidth("*");
        itemPriceBaseNet.setVisibleWhen(new AdvancedCriteria("showDiscount", OperatorId.EQUALS, true));
        items.add(itemPriceBaseNet);

        TextItem itemPriceBase = new TextItem("price_base");
        itemPriceBase.setWidth("*");
        itemPriceBase.setVisibleWhen(new AdvancedCriteria("showDiscount", OperatorId.EQUALS, true));
        items.add(itemPriceBase);

        TextItem itemDiscount = new TextItem("discount");
        itemDiscount.setWidth(70);
        itemDiscount.setHint("%");
        itemDiscount.setEndRow(true);
        itemDiscount.setVisibleWhen(new AdvancedCriteria("showDiscount", OperatorId.EQUALS, true));

        FloatRangeValidator itemDiscountValidator = new FloatRangeValidator();
        itemDiscountValidator.setMin(0);
        itemDiscountValidator.setMax(100);
        itemDiscount.setValidators(itemDiscountValidator);
        items.add(itemDiscount);

        CustomValidator validatorNoZero = new CustomValidator() {
            @Override
            protected boolean condition(Object value) {
                try {
                    return Float.valueOf(value + "") > 0;
                } catch(NumberFormatException e) {} catch(NullPointerException e) {} catch(RuntimeException e) {};
                return false;
            }
        };

        TextItem itemPriceNet = new TextItem("price_net");
        itemPriceNet.setEndRow(false);
        itemPriceNet.setWidth("*");
        items.add(itemPriceNet);


        TextItem itemPrice = new TextItem("price");
        itemPrice.setStartRow(false);
        itemPrice.setWidth("*");
        items.add(itemPrice);

        SpinnerItem itemQuantity = new SpinnerItem("quantity");
        itemQuantity.setDefaultValue(1);
        itemQuantity.setMin(1);
        itemQuantity.setStep(1);
        itemQuantity.setWriteStackedIcons(true);
        itemQuantity.setEndRow(true);
        itemQuantity.setValidators(validatorNoZero);
        items.add(itemQuantity);

        StaticTextItem itemTotalNet = new StaticTextItem("total_net");
        itemTotalNet.setDecimalPad(2);
        itemTotalNet.setEndRow(false);
        itemTotalNet.setWidth("*");
        items.add(itemTotalNet);

        StaticTextItem itemTotal = new StaticTextItem("total");
        itemTotal.setDecimalPad(2);
        itemTotal.setEndRow(true);
        itemTotal.setWidth("*");
        items.add(itemTotal);

        StaticTextItem itemTotalProfit = new StaticTextItem("total_profit");
        itemTotalProfit.setDecimalPad(2);
        itemTotalProfit.setEndRow(true);
        itemTotalProfit.setWidth("*");
        items.add(itemTotalProfit);

        return items;
    }

    public void setReadOnly(ArrayList<FormItem> items) {
        for (FormItem item : items) {
            if (item instanceof StaticTextItem || item instanceof BoxHeaderItem) {
                continue;
            }
            if (item.getAttributeAsObject("readOnlyWhen") != null) {
                AdvancedCriteria criteria = new AdvancedCriteria(OperatorId.OR, new Criterion[]{item.getReadOnlyWhen(), new AdvancedCriteria("status", OperatorId.IN_SET, new String[]{"finished", "canceled"})});
                item.setReadOnlyWhen(criteria);
            } else {
                item.setReadOnlyWhen(new AdvancedCriteria("status", OperatorId.IN_SET, new String[]{"finished", "canceled"}));
            }
        }
    }

Steps to reproduce:
- Select first record
- Click twice on showDiscount BooleanItem
- Select third record

Or click ones on showDiscount and then switch records few times.

It's connected to this code in change handler:

            form.setValue("price_base_net", "");
            form.setValue("price_base", "");
            form.setValue("discount", "");

If I change this to:

            form.setValue("price_base_net", (String)null);
            form.setValue("price_base", (String)null);
            form.setValue("discount", (String)null);

it works fine. But I think it's worth checking.

By the way, not related but another surprising behavior is that this code is causing Exception even when item is FloatItem.

form.setValue("price_base_net", (Float)null);

3. I believe you did but as I'm testing it solved problems with editRecord() and not other scenarios when multiple calls to processRules are happening.
I think some kind of scheduler would help to solve this problem. I now code I send was not sufficient to solve the problem but I was only trying to demonstrate the idea.

Some other scenarios when multiple processRules are happening:

3a. In above scenario put from keyboard just one letter in name field.
processRules is called 4 times - see attached profile: Profile_One_Key.png

3b. Change field type to any other value: 5 times.

I now above test case works fast with this scenarios but rules can be much more.

3c. On my real life app changing showDiscount item still calls processRules 18 times - and this is after some optimization with setValues() instead of setValue().

3d. On base scenario with just click on change record on ListGrid processRules is called 5 times.
On real app it's still called 14 times - I've attached profile.
In my app I used Rules not only for DynamicForm items but also to buttons in ToolBar.

So I think there is still space for improvement. Functionality of whenRules is great and I've got some more ideas to optimize some processes in my applications but I worry that it will cause performance problems when they will be used in whole application.
Please consider creating some kind of scheduler.

Best regards
Mariusz Goch

[Isomorphic]

1) good to hear

2) you can use editRecord() if what you want is the documented behavior of editRecord(), or setValues() if what you want is the documented behavior of setValues(). The point is, if you are going to modify multiple values at once, use an API that allows modifying multiple values at once, as this gives the framework the chance to optimize.

3) we already do something a bit more sophisticated

[pH4Lk0n]

1. I'm working on it. I've got a potential hit. It's probably linked with calculations i'm doing.
I've found a place where into DataSourceFloatField is set empty string.
I will post a test case.

2. Ok. But I need to pass all values. So first getValues(), modify some, and then setValues() ??
But this Form is linked DataSource and according to docs editRecord() is suggested??

3. 50ms was just a guess. Probably 0 delay will work just fine - just to wait for current event flow finishes.
Example in JS probably will explain it better.

<script>
  var processRulesTimeout;

  function processRules() {
    console.log("processRules()");
  }
  function callEvent() {
    if (processRulesTimeout) {
      clearInterval(processRulesTimeout);
    }
    processRulesTimeout = setTimeout(() => {
      processRules();
    }, 0);
  }

  function start() {
    for (let i=0; i<20; i++) {
      console.log('callEvent', i);
      callEvent();
    }
  }

start();

</script>

Even when callEvent() was called 20 times processRules() will be called only ones after loop is ended.
This could probably limit processRules() just to 1 execution per event. Even when editRecord() calls it only one there are also some call on redraw(). So with this approach we could go down from 4 calls in previous test case to 1. This should also cover all scenarios not only the ones that have a workaround.

Best regards
Mariusz Goch

[Isomorphic]

1) can you show a test case in which this call chain happens?

2) call setValues() instead of individually calling setValue() each time. This is also more efficient even outside of *When rules

3) we're note sure what you mean about your suggestion. Are you saying why would we set an immediate timer vs wait 50ms? If so, what is the advantage of waiting 50ms?

[pH4Lk0n]

Hi,

I've checked today's build 2023-07-10 (first one after July 8).

1. Basically it's much better in both test case and original application. Looking at profiles during editRecord() processRules are called only ones.

2. But there is a scenario in my application that calls processRules multiple times.
From what is worse it's called from another processRules.

I've attached screenshot on this scenario.
As you can see on screenshot during processRules during getValues(), saveValue() is called which again triggers processRules.

That was not the case before. I've just compared to previous version and in older version I cannot populate this, so it's new.

In normal case on change selected record processRules are called 4 times - one time on editRecord() but in this particular scenario it's called 20 times and times are again jumping from 25ms to 450ms.

Strange thing it's not happening from the start.
It starts when I change BooleanItem value (even without saving) and then start to switch between records.

2. Another problem that still remains is multiple processRules calls when calculating other items.
I've got some handlers that calculates several other values and uses form.setValue() to update these items.
Is there a better way to do this??

3. As I see you took different approach than delaying processRules to perform after minimal timeout. Was there a particular implementation problem with my suggestion?

Best regards
Mariusz Goch

[Isomorphic]

Changes have been made to reduce the number of times rules are processed when DynamicForms are initialized and for DynamicForm and ValuesManager editRecord() calls. This should improve your performance significantly based on your test code. These changes will be in 13.0 and later releases for builds starting July 8.

Please let us know your results.

[Isomorphic]

Thanks for the clear test case. We're checking it out - we suspect we'll be able to eliminate most of the problem by running rules only once per editRecord() / setValues() call rather than responding to each individual formItem.setValue() call that is part of the overall editRecord() call.

We'll post here with updates.