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

Bad performance #1601

Closed
lpinca opened this issue Sep 30, 2018 · 18 comments
Closed

Bad performance #1601

lpinca opened this issue Sep 30, 2018 · 18 comments
Assignees
Labels
feature New functionality or improvement

Comments

@lpinca
Copy link

lpinca commented Sep 30, 2018

Context

  • node version: v10.11.0
  • joi version: joi@13.7.0
  • environment (node, browser): node
  • used with (hapi, standalone, ...): standalone
  • any other relevant information:

What are you trying to achieve or the steps to reproduce ?

Joi.validate() seems to have some performance issues. Here is a benchmark with a very simple schema.

const Ajv = require('ajv');
const Benchmark = require('benchmark');
const Joi = require('joi');

const ajv = new Ajv();

const ajvSchema = {
  type: 'object',
  additionalProperties: false,
  properties: {
    id: {
      type: 'string',
      minLength: 1
    },
    level: {
      type: 'string',
      enum: ['debug', 'info', 'notice']
    }
  },
  required: ['id', 'level']
};

const validate = ajv.compile(ajvSchema);

const joiSchema = Joi.object({
  id: Joi.string().required(),
  level: Joi.string()
    .valid(['debug', 'info', 'notice'])
    .required()
}).unknown(false);

const options = { convert: false };

const suite = new Benchmark.Suite();
const value = { id: '1', level: 'info' };

suite.add('Joi', function() {
  Joi.validate(value, joiSchema, options);
});
suite.add('Ajv', function() {
  validate(value);
});
suite.on('cycle', function(event) {
  console.log(event.target.toString());
});
suite.on('complete', function() {
  console.log('Fastest is ' + this.filter('fastest').map('name'));
});

suite.run({ async: true });

Which result you had ?

node index.js 
Joi x 31,497 ops/sec ±3.72% (75 runs sampled)
Ajv x 10,543,398 ops/sec ±0.57% (86 runs sampled)

What did you expect ?

Better performance. Am I doing something wrong?

Thank you.

@martabacc
Copy link

hi @lpinca , this issue is interesting.

I'm new to this, but I'd love to contribute in optimizing this (since we are using it in our project).
What do you think in mind to solve this?

Is there any particular part that consume most resources compared to others?

@lpinca
Copy link
Author

lpinca commented Oct 17, 2018

@ronayumik I honestly don't know, I'm not familiar with joi codebase and I didn't profile the code to find bottlenecks.

@kanongil
Copy link
Contributor

I don't see the issue here. You are comparing apples and oranges, and a factor of 3 vs a performance optimized application specific validator seems reasonable.

That being said, there are likely opportunities to improve the performance of Joi, especially for specific corner cases.

@lpinca
Copy link
Author

lpinca commented Oct 17, 2018

You are comparing apples and oranges, and a factor of 3 vs a performance optimized application specific validator seems reasonable.

Agreed but the factor is 300 not 3. My intention is not to compare joi with ajv. The point is that 31k ops/sec for a dead simple schema like the one in the issue description seem low. I would expect performance to be at least an order of magnitude better.

@kanongil
Copy link
Contributor

Ah, yes I misread, and you point is correct. Though, I still suspect it would require architectural changes to achieve something like that.

@yonjah
Copy link

yonjah commented Oct 19, 2018

You are comparing apples and oranges, and a factor of 3 vs a performance optimized application specific validator seems reasonable.

@kanongil I'm not sure I understand your comment but I'm not familiar with ajv. Can you explain what were they compromising compared to joi that allows them to reach those performance optimizations ?

@Marsup
Copy link
Collaborator

Marsup commented Oct 19, 2018

ajv is a compiler, it actually creates adhoc code to validate schemas, I doubt joi will ever reach this kind of performance without doing something similar which is a major undertaking. That said, we can always try to squeeze as much performance as possible.

@Marsup
Copy link
Collaborator

Marsup commented Oct 19, 2018

Not that I know, even though I think joi is very rarely the culprit of any service. I understand this kind of discussions, but micro-benchmarks are meaningless when you consider your application as a whole, the point is whether the value provided by the library is what you're looking for or not. If you really need speed, you wouldn't be doing JavaScript.

I'll investigate this example whenever I have some time. Already by inlining the options you double the performance, but that's still not enough.

@lpinca
Copy link
Author

lpinca commented Oct 19, 2018

I think joi is very rarely the culprit of any service. I understand this kind of discussions, but micro-benchmarks are meaningless when you consider your application as a whole, the point is whether the value provided by the library is what you're looking for or not. If you really need speed, you wouldn't be doing JavaScript.

I agree and that is generally true but frameworks like polka or fastify can do 50k reqs/sec on a single unclustered server and joi is barely faster than that so it can be a limiting factor. This is why I think performance should be at least an order of magnitude better and why the issue was opened in the first place.

@Marsup
Copy link
Collaborator

Marsup commented Oct 19, 2018

Using a bare server with no business logic in it is a useless comparison. Start adding some actual workload on that and see which share of the performance is taken by joi or even fastify and you'll see things differently. I'm not saying joi can't be faster, just putting things in perspective.

@Marsup
Copy link
Collaborator

Marsup commented Oct 19, 2018

To avoid repeating something that was eloquently said: https://hueniverse.com/when-500-faster-is-garbage-553121a088c3. That's not to say I don't want to reach the best possible performance, I'll take whatever's actionable, I just won't enter into a competition with other libraries.

@lpinca
Copy link
Author

lpinca commented Oct 19, 2018

Yes but it depends on the use case. I'm analysing the performance of a web app in production and in some routes the biggest amount of CPU time is spent on validation due to joi which is a shame. This is clearly visible in the flame graph. I really like joi and its API, I wish it was faster, that's all.

@Marsup
Copy link
Collaborator

Marsup commented Oct 19, 2018

So do I, but we need to find something actionable about it.

@Marsup
Copy link
Collaborator

Marsup commented Oct 21, 2018

Commit a1a79b3 should help a bit.

Your version Inlined options (for joi only)
Node 8 10 8 10
Joi 54,190 82,922 237,350 449,119
Ajv 13,469,330 28,562,774
Joi (new commit) 112,256 210,549 629,125 823,476

It's still not great but already doubles the performance for this specific use case.

My time being finite, it'll be an on and off job, so if it's really important for you, help me work on it or go with ajv.

@lpinca
Copy link
Author

lpinca commented Oct 21, 2018

Thank you @Marsup.

@Marsup Marsup self-assigned this Nov 3, 2018
@Marsup Marsup closed this as completed in 8adf1d8 Nov 3, 2018
@Marsup
Copy link
Collaborator

Marsup commented Nov 3, 2018

This last commit kind of closes this issue as a monitoring tool for this case. Feel free to contribute either in terms of new benchmarks or performance improvements. I'll make sure to keep things moving in this area but I can't promise a big break in performances anytime soon. I don't think it's necessary to keep this issue open as it's as endless as can be.

@kanongil
Copy link
Contributor

kanongil commented Nov 3, 2018

Awesome. I will see if I can dig up some of my old tests, and add them to the suite.

@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement
Projects
None yet
Development

No branches or pull requests

6 participants