-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
performance issue of 4.0.0&4.0.1 #2809
Comments
I've noticed some performance degradation, but it seems like you have some more in-depth info. I'll investigate but seeing some sample code would really help |
@losogithub do you see the following warning message when you starting your app?
If you do, then it's not a mongoose issue. And it would explain your performance issues. |
@lbeschastny I get that message #2744 (comment) |
@AntouanK Make sure you have all the build tools installed. Which platform are you on? If you're on OS X, that will be |
@lbeschastny I had fixed the c++ bson extension issue before I encountered this performance issue. Thank u all the same. |
@paambaati I'm on CoreOS but I use docker containers ( it doesn't really matter what OS the container is in ). I can get |
Turns out I also need the You can check out the |
Oh yeah. I'd recommend 2.7.6 or higher.
|
I confirm that I experience big app slowdown - but without bson warning. |
Can you provide me an example of an operation or block of code that you're experiencing a big performance degradation for? Difficult to debug when I don't have anything to go on :) FWIW I was investigating why my tests became much slower in 4.0.0-rc3 and bisected it down to mongodb-js/mongodb-core@2dd5a5b, which adds a long timeout for some connection synchronization. If you have benchmarks that create a new connection every time you may notice a performance hit due to that. |
It is overall slowdown and memory overload. It is very difficult to understand which problem it is - mongoose or driver because one depends on another. P.S. I create persistent connection to replica set once at start of app with keepAlive option as API suggests. |
I just saw an issue on nodejs driver: |
Re: mongoose 4 with mongodb driver 1.4, should be possible but will take a fair amount of work to set that up. I think a better approach may be to create a new benchmark suite, because right now we don't really have a working one for mongoose. I'll keep this issue open to track, we'll see how NODE-420 goes. Also, are you seeing performance degradation just against replica set or against standalone as well? |
Oops. I checked slowdown on standalone. I did not check replica set. |
Ah I thought you were running against a replica set. Will track the node issue and work on my own benchmarks to see if I can repro. In the meantime, can you provide me a standalone code sample that demonstrates the performance reduction you're seeing? Would be very helpful :) |
exports.validate = function(token, callback) {
}; |
@AHDPuK could you please share the code you've used to run your benchmarks? |
@lbeschastny
|
I created my own benchmark to reproduce this performance issue: https://github.com/lbeschastny/mongoose-benchmark My benchmark creates a MongoDB document with 3000 randomly-filled fields (Strings, Numbers and Dates) and uses this document to benchmark Here are my results:
I found no trace of performance degradation between versions @AHDPuK, @losogithub, could you please provide more information about your performance problems, including information about your system (OS, node.js and MongoDB versions, etc)? Could you also run my benchmark on your system to check if it'll reproduce your problem? It'll be really helpful if we will see some complete code example to reproduce aforementioned performance degradation, including database initialization. |
I can guess what is the central problem in my case. This is a response document size. I keep user sessions in collection "sessions". This is only collection with extra large documents because I need to store cached info for each session and not generate data with each request. Size of one document is approximately 1Mb. Exactly post processing of response makes a difference in my case. Schema definition: |
P.S. I run it with the same results I described before module n is not supported on Windows. |
@AHDPuK tahnx! I replaced my dummy model and randomly-generated data with your example and thus reproduced performance degradation. I created new branch for you data: Here are my new results:
|
@AHDPuK I played a bit with your data and boiled it down to the |
@AHDPuK with your help I found a minimal example reproducing this performance degradation: var Schems = new mongoose.Schema({
data: {
type: [{
s: String
}]
}
}); I also added it to my benchmark: lbeschastny/mongoose-benchmark@f492557 Here are my benchmarking results for a document containing 1000-elements long array of objects (aka sub-documents):
|
Thanks for the repro guys, looks like document arrays are the problem. Your help is much appreciated, I'll look into this over the weekend / early next week. |
@vkarpov15 I run another couple of benchmarks, truing to get to the bottom of this issue. I found that
Hope you'll find out what's wrong here and fix it. |
@vkarpov15 I narrowed it down to DocumentArray#cast function:
|
Thanks for the extra info. I don't quite have time to dive into this today but I'll take a look over the weekend if you haven't resolved it by then. PRs are most welcome :) |
I narrowed it even more, not to the
|
@vkarpov15, found it! The most of it was caused by 6ce5a6c commit where you moved a heavy |
@vkarpov15 just realized that you already fixed it in #2821. |
@vkarpov15 I found another troublemaker - it's a Document#$__registerHooksFromSchema. The problem is that Mongoose sets pre-save and pre-validate hooks for each sub-document in an array. Not sure how to fix it, though. I think, you should try to do all this validation routine without heavy hooks. Or, you should try to figure out how to move initialization of those build-in hooks out from Document constructor. Even if there is no other way to deal with user-defined hooks, there must be a way to move build-in hooks initialization to the prototype level. |
Looks like that's all. All our performance troubles was caused by those two things, and one of them is already fixed with #2863 PR. @vkarpov15 I'll leave the rest to you. |
Thanks for finding these issues. Good catch re: my mistakes with 6ce5a6c, that was far from my best work. The second issue will be ameliorated slightly by #2894, which removes the extra pre('validate') hook (its redundant). But you're right, we should probably avoid adding that pre('save') validate to sub-documents, that's unnecessary because the first thing the hook does is check if its a subdoc. And I agree about not registering hooks for a document every single time one is created. That's one of the numerous reasons why the plan is to ditch the hooks library entirely and instead use kareem to have more fine-grained control over what context hooks run in. However, in the wise words of Jesse Ventura in Predator, hooks are dug in like an Alabama tick and removing the library is more of a long-term project than a quick bug fix. |
Gonna mark this as closed since @lbeschastny solved the regression - the hooks issue goes back to 3.8 and should be fixed by #2754 |
@vkarpov15 let's be honest, it was already fixed by #2863 PR by the time I've started investigating the problem. 🐱 |
After upgrading from 3.9.* to 4.0.0&4.0.1, my node process's CPU usage increased by more than 200%.
Each http request which uses mongoose costs cpu % 3 times than before.
And when I downgrade back to 3.8.25, everything is OK.
The text was updated successfully, but these errors were encountered: