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

Fill and humanize labels at definition time #494

Closed
tmeasday opened this issue Nov 5, 2015 · 7 comments
Closed

Fill and humanize labels at definition time #494

tmeasday opened this issue Nov 5, 2015 · 7 comments

Comments

@tmeasday
Copy link

tmeasday commented Nov 5, 2015

Ok, moving over from meteor/guide#53 (comment)

This schema:

new SimpleSchema({a: {type: String}, b: {type: String}, c: {type: String}})

Takes ~2.5s to validate 10k documents of the form {a: 'a', b: 'b', c: 'c'}.

If I add a label to each field definition (to bypass the call to S.humanize()), that time drops to ~325ms.

@aldeed - would you be interested in a PR that helps with this problem? Some ideas of how to solve it:

  1. Don't call humanize on the label until later when we actually need it
  2. Cache the humanized labels (call it at definition time?)
@aldeed
Copy link
Collaborator

aldeed commented Nov 9, 2015

Good ideas. I will get this done.

@tmeasday
Copy link
Author

I'm happy to send a PR too if you point me in the right direction

@aldeed
Copy link
Collaborator

aldeed commented Nov 13, 2015

#240 can be done as part of this, too. In SS constructor, we will inflect any missing labels unless humanizeAutoLabels option is false, in which case we will just set the label to the key. All other code can rely on label being set already.

@aldeed aldeed changed the title Performance of schema checks Fill and humanize labels at definition time Nov 13, 2015
@aldeed aldeed mentioned this issue Nov 13, 2015
2 tasks
@aldeed
Copy link
Collaborator

aldeed commented Nov 13, 2015

Also adding #494 to this issue:

Before humanizing, check whether SimpleSchema.defaultLabel or mySS.defaultLabel is a function and use that instead for missing labels. Add example to readme like this for i18n:

SimpleSchema.defaultLabel = function (key) {
  return i18n.t(key);
};

@marcodejongh
Copy link
Contributor

@tmeasday Do you still have to benchmark readily available? Could you run it on the current version of master? I removed string.js in #480 the new humanize function is a bit simpler I'm curious to how much of a difference it makes performance wise.

@tmeasday
Copy link
Author

Hey @marcodejongh - I'm not sure exactly what I was running previously, but just trying the following code:

const s = SimpleSchema({a: {type: String}, b: {type: String}, c: {type: String}});

var start = new Date();

_.times(10000, () => {
  check({a: 'a', b: 'b', c: 'c'}, s);
});

new Date() - start;

Was previously taking ~1100ms in the browser console, now it takes ~550ms. So that's a nice 2x speedup.

In the server console it was taking ~4700ms and now it's taking ~1200ms, so that's even better!

@aldeed
Copy link
Collaborator

aldeed commented Aug 24, 2016

Thanks for your patience. SimpleSchema 2.0.0-rc.1 is now released and should fix this bug.

There are a number of breaking changes when updating to 2.0, so be sure to check out the change log. If you use aldeed:collection2, you will need to use 2.10.0 or higher of that package in order to use SimpleSchema 2.0. If you use autoform, it is not yet updated to work with SimpleSchema 2.0, but hopefully soon.

SimpleSchema is now an isomorphic NPM package, so you can check out the updated readme and file issues over at the other repo. The Meteor wrapper package will exist for now but eventually I will probably deprecate support for it.

This is still a beta/RC and I do expect people will find issues, so use with caution. Production use is not yet recommended. That said, there are more and better unit tests than ever before, and the codebase should be much easier for others to read through and debug quickly.

@aldeed aldeed closed this as completed Aug 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants