Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: AutoForm performance? #70

Closed
mdorn opened this issue Jul 14, 2016 · 17 comments
Closed

Question: AutoForm performance? #70

mdorn opened this issue Jul 14, 2016 · 17 comments
Assignees
Labels
Type: Feature New features and feature requests

Comments

@mdorn
Copy link

mdorn commented Jul 14, 2016

I'm using AutoForm with minimal configuration on a Simple Schema that contains about 10 fields (mostly text fields, but one is an object that populates about 12 select boxes). I'm using AutoField for all of them. I've noticed that when I type in one of the text boxes, there's a very noticeable lag, and the CPU usage on my computer spikes significantly.

As I've experimented, I've noticed that any time there's more than 8 or 10 fields on the form, performance suffers badly.

Is there anything that I can do to help this situation besides not using AutoForm ?

I've actually cobbled together a simple test form that reproduces the problem: if I add 10 items to "categories" using the form below, typing into the "name" field becomes very slow and CPU-intensive:

import { SimpleSchema } from 'meteor/aldeed:simple-schema';

import _ from 'lodash';
import React from 'react';
import { AutoForm, ErrorsField, AutoField, SubmitField } from 'uniforms-unstyled';

const TestSchema = new SimpleSchema({
  name: { type: String },
  categories: { type: [String], defaultValue: [] },
});

export const getForm = () => {
  const fields = ['name', 'categories'];
  const autoFields = _.map(fields, field => <AutoField name={field} key={field} />);
  return (
    <AutoForm
      schema={TestSchema}
      onSubmit={doc => (console.log(doc))}
    >
      <ErrorsField />
      {autoFields}
      <SubmitField />
    </AutoForm>
  );
};
@serkandurusoy
Copy link
Contributor

serkandurusoy commented Jul 14, 2016 via email

@radekmie radekmie added the Type: Feature New features and feature requests label Jul 14, 2016
@mdorn
Copy link
Author

mdorn commented Jul 14, 2016

@serkandurusoy - even if you have a single form with a single array field, in my example once the categories array has more than a handful of values, you will experience the performance issue.

When you say "handle the individual pieces of data," do you mean that you stopped using AutoForm and/or AutoField?

@radekmie radekmie self-assigned this Jul 14, 2016
@radekmie
Copy link
Contributor

To be honest, I was pretty sure, that I will release v1.0.0 a while ago, then I wanted to focus on optimizations... There are few easy to achieve performance gains, so I'll work on it in the near future, but after v1.0.0 it will be the main goal.


As of #31, there's AutoFields component.

<AutoFields fields={['name', 'categories']} />

@mdorn
Copy link
Author

mdorn commented Jul 14, 2016

Thanks @radekmie for the quick response. I'm still getting up to speed with the library (which otherwise seems excellent by the way), so do you have any advice for a temporary workaround? I don't need real-time validation, for example; validating onSubmit would be fine with me -- is there any way I can disable real-time validation and achieve better performance that way?

@radekmie
Copy link
Contributor

By default, ValidatedForm (and so AutoForm) validates onChangeAfterSubmit. If you want to validate only onSubmit, then use it like this:

<AutoForm validate="onSubmit" schema={...} onSubmit={...} />

(okay, it's not well documented, but it's on this diagram)

@mdorn
Copy link
Author

mdorn commented Jul 14, 2016

Thanks, I did try that earlier, but it didn't have the expected impact on performance.

@radekmie
Copy link
Contributor

Well, that means, that validation is not a problem.

@serkandurusoy
Copy link
Contributor

@mdorn I do use autoform as is but in all fairness, my usecase did not include array fields that would grow too big and it was mainly used as an update form, so I was able to split to big form into small forms which dealt with relevant parts of the docuement.

Something like, a form updates user's name and age, another form updates the address, another updates some other profile field, they are all autosave so there's no button involved, which in turn creates the perception of a single profile edit form.

but yeah, performance is an issue :(

@radekmie
Copy link
Contributor

radekmie commented Jul 14, 2016

I've played a little with react-addons-perf and implemented quite complex shouldComponentUpdate in BaseField. It is a huge speed up - tests are 3~4 times faster. Also, currently ListField is the heaviest one, because of additional ListAddField and ListDelField - those can be optimized, but (at least for me) current improved performance is enough.

@mdorn, @serkandurusoy
Please, test it - it's a huge improvement, so it would be nice, to release it as soon as possible.

Note: This depends on some (maybe incorrect) assumes, like label or placeholder won't change without value change.

@mdorn
Copy link
Author

mdorn commented Jul 14, 2016

@radekmie - thanks for the quick turnaround on this. Subjectively (without doing any real profiling, etc. -- I'm still pretty new to React and Meteor), it does seem a bit faster.

However in the conditions I describe (1 String field and 10+ text boxes on the screen as part of a single Array field), typing in one of the text boxes continues to feel quite sluggish. If a fast typist (e.g. 90 words per minute) types a sentence of 140 characters or so, it may take 2-3 seconds to see all of the words in the input box. This is true even if I remove any Array fields so that ListFields are not used (e.g. a form with 4 string fields with no allowedValues set, 2 others with allowedValues so they render as select boxes, and one Object field with an external schema so that it renders as 10-12 select boxes).

I've not tried it on multiple computers yet -- FWIW here are the specs of the computer I'm working on:

MacBook Pro (15-inch, Mid 2010)
2.4 GHz Intel Core i5
8 GB 1067 MHz DDR3

@radekmie
Copy link
Contributor

Another optimization done.

@mdorn
Could you share your schema? It will be easier for me to test it.

@serkandurusoy
Copy link
Contributor

@radekmie here's a schema where we had problem with

Especially the arrays were problematic, for example contact emails!

In fact, even after separating the schema into smaller schemas in individual forms, the contact emails schema on its own, when used with autoform, becomes really slow after adding a few emails into the array

Meteor.users.masterProfile.schema = new SimpleSchema({
  userId: {
    type: String,
    index: 1,
    unique: true,
    regEx: SimpleSchema.RegEx.Id,
  },
  accountEdited: {
    type: Boolean,
    defaultValue: false,
  },
  'name.first': {
    type: String,
    max: 256,
  },
  'name.last': {
    type: String,
    max: 256,
  },
  'organization.name': {
    type: String,
    max: 256,
    optional: true,
  },
  'organization.role': {
    type: String,
    max: 256,
    optional: true,
  },
  slug: {
    type: String,
    index: 1,
    unique: true,
    denyUpdate: true,
    autoValue: function() {
      const first = this.field('name.first');
      const last = this.field('name.last');
      if (this.isInsert) {
        if (first.isSet && last.isSet) {
          const userSlug = slugify(`${first.value} ${last.value}`);
          const usersCount = parseInt(Meteor.users.masterProfile.find({ slug: userSlug }).count());
          return `${userSlug}${usersCount > 0 ? '-' + usersCount + 1 : ''}`;
        }
      } else {
        this.unset();
      }
    }
  },
  avatarUrl: {
    type: String,
    optional: true,
  },
  birthDay: {
    type: Date,
    optional: true,
  },
  contactEmails: {
    type: [Object],
    optional: true,
    custom: function() {
      if (this.isSet) {
        if (this.value.length !== _.uniq(_.pluck(this.value, 'address')).length) {
          return 'notUnique';
        }
      }
    },
  },
  'contactEmails.$.address': {
    type: String,
    optional: true,
    regEx: SimpleSchema.RegEx.Email,
  },
  'timeZone.identifier': {
    type: String,
    allowedValues: TIME_ZONE,
  },
  'timeZone.setManually': {
    type: Boolean,
    defaultValue: false,
  },
  'phones.$.countryDialCode': {
    type: String,
    allowedValues: DIAL_CODE,
    optional: true,
  },
  'phones.$.number': {
    type: String,
    max: 256,
    optional: true,
  },
  'address.street': {
    type: String,
    max: 256,
    optional: true,
  },
  'address.city': {
    type: String,
    max: 256,
    optional: true,
  },
  'address.zipCode': {
    type: String,
    max: 256,
    optional: true,
  },
  'address.country': {
    type: String,
    allowedValues: _.pluck(COUNTRY, 'name'),
    optional: true,
  },
  'siteLinks.$.name': {
    type: String,
    max: 256,
    optional: true,
  },
  'siteLinks.$.url': {
    type: String,
    regEx: SimpleSchema.RegEx.Url,
    optional: true,
  },
  'socialLinks.facebook': {
    type: String,
    regEx: SimpleSchema.RegEx.Url,
    optional: true,
  },
  'socialLinks.linkedIn': {
    label: 'LinkedIn',
    type: String,
    regEx: SimpleSchema.RegEx.Url,
    optional: true,
  },
  'socialLinks.twitter': {
    type: String,
    regEx: SimpleSchema.RegEx.Url,
    optional: true,
  },
  'socialLinks.googlePlus': {
    type: String,
    regEx: SimpleSchema.RegEx.Url,
    optional: true,
  },
})

@radekmie
Copy link
Contributor

Okay, thanks - I'll play with it.

radekmie added a commit that referenced this issue Jul 17, 2016
@zeroasterisk
Copy link
Contributor

@radekmie nice work on optimizations - let us know how the hunt for performance increases goes

@radekmie
Copy link
Contributor

I've just released 1.0.0-rc.25. It contains few bugfixes and enhancements, but the most important are optimizations. I've tried your schema, @serkandurusoy - it's immensely huge and it almost melted my computer while holding a single key in an input of generated form. Right now, after some work (and checking performance with react-addons-perf) it's okay. I've tried some heavy load (about 5 items in each array of that schema) and it was good. Now it's about ~200ms wasted during typing - it was ~6000ms (!).

Further optimizations are planned, but current version is good enough (for me of course) - now I want to close few more issues and finally release 1.0.0.


I'll close this issue, as the main problem with CPU intensive forms is solved - don't worry, more optimizations are coming.

@zeroasterisk
Copy link
Contributor

FTW!!!

On Mon, Jul 18, 2016, 3:32 PM Radosław Miernik notifications@github.com
wrote:

Closed #70 #70.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#70 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AABbfpYDSLxFhJvrdfcvGFFFQwqGMg8Iks5qW9SmgaJpZM4JMbRV
.

@mdorn
Copy link
Author

mdorn commented Jul 18, 2016

Thanks @radekmie -- HUGE improvement! Large numbers of items rendered as ListAddField will still slow things down some, but I do think this is "good enough" for now.

@Monteth Monteth added this to Closed in Open Source (migrated) Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New features and feature requests
Projects
Archived in project
Development

No branches or pull requests

4 participants