Validations slow performance

[jakesjews]

Environment

• Ember Version: 2.9
• Ember CLI Version: 2.9
• Ember CP Validations Version: Master

Steps to Reproduce

I've been running into a lot of performance issue on a grid which can create many objects. I narrowed down the issue to slow performance in checking the isValid properties on each object. At first I assumed that I had some slow custom validators but after trimming them down to only use presence checks the issue still remained. Running the following test takes around 700ms on my laptop with only presence validations. This test was added to the suite on the master branch to make sure I didn't have anything in my code causing the issue.

test('load test', function(assert) {
  this.register('validator:presence', PresenceValidator);

  let Validations = buildValidations({
    a: validator('presence', true),
    b: validator('presence', true),
    c: validator('presence', true),
    d: validator('presence', true),
    e: validator('presence', true)
  });

  let Klass = Ember.Object.extend(Validations, {
    a: null,
    b: null,
    c: null,
    d: null,
    e: null
  });

  let items = [];
  for (let i = 0; i < 50; i++) {
    let obj = setupObject(this, Klass, {
      a: i,
      b: i,
      c: i,
      d: i,
      e: i
    });
    items.push(obj);
  }

  console.time('test');
  Ember.A(items).mapBy('validations.isValid');
  console.timeEnd('test');
  assert.ok(true);
});

[offirgolan]

@jakesjews thanks for taking the time to open up this issue. Can you modify the test to check the performance of subsequent checks?

The reason you are hitting performance issues is because when you do a get call to any property on the validations object on a fresh instance, thats when the entire validations structure is built. This means that every new instance has some initial overhead but that overhead should be minimal on subsequent property checks.

The library itself is designed to be optimal. All internal validations classes are created once per class not per instance. The entire validations class is lazily created as well as all its child properties.

Can you give me some context on what you are trying to achieve? Maybe we can figure out a workaround for this.

[offirgolan]

Also, 700ms for 50 objects seems a bit high. I'll try to investigate this further. @jakesjews can you also post you laptop specs.

cc @stefanpenner

[jakesjews]

Thanks for the reply and the great library!

I have a late 2011 macbook Pro with a quad core 2.4 i7 processor and 16GB of ram.

The application with the performance issues is an interface to program logic controllers on assembly lines. I have a grid in the application which builds part configuration objects for a given task (like a robot arm) for many types of parts which could come through. Adding a row to this grid can take 3 or 4 seconds since there are probably around 10 validations defined on each of the configuration objects.

From some poking around it looks like the performance issues aren't in the mixin definition but happen somewhere in here

I was pretty certain the issue was some sort of slow validator we had defined but after deleting most of them and replacing them with presence checks the issue still persisted.

[jakesjews]

I tried running the load test on the latest 2.x branch and it looks like its a little over 2 times faster in 2.x. I'll see if I can find out where the regression happened.

[offirgolan]

@jakesjews so I took some time to dive into this a bit and it seems as though my deductions were correct. A lot of work has been done in 3.x to move the majority of the overhead to the first get of the validations object of an instance. From my investigations, a lot of the overhead comes from the first get requests to the CPs. After that, all other validations checks are very quick. I'll keep digging into this and see what else can be done.

[jakesjews]

@offirgolan I think I've narrowed down the bulk of the performance issue to the buildOptions function https://github.com/offirgolan/ember-cp-validations/blob/cb4aa57b6e1f3e1234f2f5a1b1ffb55b26a0a4e4/addon/validators/base.js#L110. It looks like any time a new object is created it is creating a new ember class for each validated field. In the test example above it is creating 250 new classes for 50 objects. I experimented a bit with caching the class creation based on arguments and saw the test time reduced by around 50%. The only issue I ran into was that it was causing issues between tests since the cache wasn't being reset. Would you be open to a PR adding some sort of service to store a cache for those classes or maybe a different better approach?

[offirgolan]

@jakesjews how exactly are you caching the class creation?

[jakesjews]

@offirgolan I think your solution is actually better but I was memoizing the arguments to buildOptions like

  buildOptions(options = {}, defaultOptions = {}, globalOptions = {}) {
    let builtOptions = mergeOptions(options, defaultOptions, globalOptions);

    // Overwrite the validator's value method if it exists in the options and remove it since
    // there is no need for it to be passed around
    this.value = builtOptions.value || this.value;
    delete builtOptions.value;

    let stringifiedArgs = JSON.stringify(builtOptions);
    let result = optionsCache[stringifiedArgs];

    if (result) {
      return result;
    }

    let OptionsClass = Ember.Object.extend(builtOptions, {
      model: computed(() => get(this, 'model')).readOnly(),
      attribute: computed(() => get(this, 'attribute')).readOnly(),

      copy(deep) {
        if (deep) {
          return OptionsClass.create();
        }

        return Ember.Object.create(Object.keys(builtOptions).reduce((obj, o) => {
          obj[o] = get(this, o);
          return obj;
        }, {}));
      }
    });

    result = OptionsClass.create();
    optionsCache[stringifiedArgs] = result;
    return result;
  }

where options cache was an object variable defined at the top.

[jakesjews]

Another possible improvement I found was to replace using getProperties with individual gets. In the chrome profiler it was showing the getProperties can't be optimized by chrome because of the way it loops over arguments. I didn't benchmark it yet so I'm not sure how much improvement it actually makes but I can check it out tomorrow.

[offirgolan]

@jakesjews can you checkout the PR I opened and add your findings? If it looks like it works well I can make a patch release.

Another possible improvement I found was to replace using getProperties with individual gets. In the chrome profiler it was showing the getProperties can't be optimized by chrome because of the way it loops over arguments.

I actually went ahead and tested this and I noticed no difference in performance.

[jakesjews]

@offirgolan its a lot faster now. The PR looks great!

[offirgolan]

@jakesjews thanks for seeing this issue through and helping me locate the source of the issue. I released v3.2.0 with the changes 😸