tag1consulting / goose

Load testing framework, inspired by Locust
https://tag1.com/goose
Apache License 2.0
737 stars 67 forks source link

Speed up user initialization #557

Closed Michael-Hollister closed 11 months ago

Michael-Hollister commented 11 months ago

When running goose tests with large amounts of goose users on my machine, it takes roughly 1 minute to initialize 1000 goose users. It appears that building each reqwest client for each goose user is an expensive operation. If we change the behavior to build only 1 reqwest client, and clone the object when creating the goose users, we can significantly increase initialization performance. With this change, I can create a few million goose users and only takes a couple of seconds to complete the initialization phase on my machine.

jeremyandrews commented 11 months ago

Wow, that’s indeed an impressive optimization. At a first glance and with minimal testing it looks great. I’ll look a bit closer shortly, and likely will merge as is. Thanks!

jeremyandrews commented 11 months ago

I've done a bit more testing.

On Linux, this is a massive improvement. Without the patch, trying to start too many users not only takes a very long time, but it uses very large amounts of memory. With the patch, it launches very quickly.

On Mac OS X (Silicon) there's no big difference with or without the patch. I'm still trying to understand why, but even if it's a Linux-only improvement it's still a nice improvement.

Michael-Hollister commented 11 months ago

Ah, that is interesting. Yeah, I am primarily using Linux for running my tests. I've also tested this on Windows, and I observe similar behavior you see on Mac OS X, where user initialization is very quick without the patch and not taking up a significant amount of memory.

I also tried using the RustLS TLS bindings on Linux (https://book.goose.rs/config/rustls.html#rustls), but seems like that had no effect with the un-patched goose version in regards to speed/memory usage.

jeremyandrews commented 11 months ago

Thanks for the clean patch and impressive optimization contribution!

jeremyandrews commented 7 months ago

Unfortunately this optimization has to be reverted. It results in the CookieJar and Headers to be shared between all clients: while this may not matter in some circumstances, it's not a good default. See this PR where I've added tests and reverted the .clone() optimization: https://github.com/tag1consulting/goose/pull/575

I'd welcome a new PR to optionally restore this optiization if it wraps the optimization in a configuration option, so you'd have to intentionally disable cookie/header support for this quick startup.

Michael-Hollister commented 7 months ago

Sure, I understand and thanks for letting me know. All of my Goose usage is done on Linux with large numbers of goose users, so this is essential for my use-case. I'll work on submitting a new PR making this a configuration option.

jeremyandrews commented 6 months ago

@Michael-Hollister I took a stab at one possible solution, can you give it a try? https://github.com/tag1consulting/goose/pull/578

I was originally thinking it could be a run-time option, but figured it was better to not even compile the related Reqwest code if it's not being used.

Michael-Hollister commented 6 months ago

@Michael-Hollister I took a stab at one possible solution, can you give it a try? #578

I was originally thinking it could be a run-time option, but figured it was better to not even compile the related Reqwest code if it's not being used.

Thanks @jeremyandrews! Tested the branch and the cargo feature approach works fine for me.