Got: Usage without caching

[timdp]

So I'm looking to use got without any caching, which seems to be the default with { cache: false }. My code calls got() hundreds of thousands of times per minute, so performance is critical.

Every call to got() currently creates a new instance of CacheableRequest, which is really a factory that gets called exactly once to produce a decorator around the raw request. Internally, that in turn creates a new instance of Keyv which then (by default) goes on to create a Map it's not going to use either. So am I misunderstanding something or does the default behavior allocate all these objects for nothing? Using the current API, I think the best I can do is pass a NullCache that doesn't allocate the Map over and over, but everything else is not configurable.

It also seems like the CacheableRequest factory is meant to be reused rather than creating a new one for every request. Would you accept a PR that memoizes the call so that every { request, cache } tuple uses the same factory?

Thanks!

[szmarczak]

So I'm looking to use got without any caching

So don't specify the cache option :)

My code calls got() hundreds of thousands of times per minute

Very bad idea.

performance is critical

Yeah, Node is very slow compared to C++ :)

It also seems like the CacheableRequest factory is meant to be reused rather than creating a new one for every request.

I think you're right.
@sindresorhus what are your thoughts about it?

[timdp]

So don't specify the cache option :)

I'm not. { cache: false } is the default.

Based on the documentation, it's not overly clear what that option does. However, if I call got(url) on a cacheable resource multiple times, it just re-requests it. This also confirms how I understood the code: it creates all those cache objects but it doesn't actually use them, and even if it did, they get recreated for the next request.

So maybe the documentation should clarify that at the very least, you need to pass { cache: someMap } to enable caching? Right now, it states:

Got implements RFC 7234 compliant HTTP caching _which works out of the box in-memory_ and is easily pluggable with a wide range of storage adapters.

That's misleading to me. But anyway, I'm not looking to enable caching personally. :slightly_smiling_face:

Very bad idea.

Why? Even without performance as an explicit design goal for got, I don't see why invoking it multiple times (whether it's ten, hundred, or hundreds of thousands) would be such a stretch. We've been doing this since version 6 and on the whole, it's working out quite well.

Yeah, Node is very slow compared to C++ :)

I never said it wasn't, but it's fast enough for my use case. I also wasn't talking about speed per se, although the object allocations do of course influence garbage collection and therefore execution time.

Anyway, independent of the language, my point was that with default settings, a bunch of throwaway objects seem to get allocated, so my suggestion would be to only allocate those conditionally. :slightly_smiling_face:

[sindresorhus]

A few extra object allocations should be negligible compared to the network request itself. I'm really curious why you're calling Got so many times per minute.

[sindresorhus]

That being said, I don't mind a PR to improve it, as long as it doesn't change any observable behavior.

[timdp]

Because we make that many outgoing requests every minute. :-)

[szmarczak]

it creates all those cache objects but it doesn't actually use them, and even if it did, they get recreated for the next request

When caching is off it doesn't cache anything, so you're wrong. But CacheableRequest is created every new request, that's a con.

I'm really curious why you're calling Got so many times per minute.

Me too. Usually you don't do that unless you do exploiting some site or crawling the web.

[timdp]

I don't think I'm wrong. I added console.log() statements at key points in the code of got, cacheable-request and keyv and then simply called got('https://www.google.com/'). This logged:

got: new CacheableRequest()
cacheable-request: this.cache = new Keyv()
keyv: new Map()
got: cacheableRequest()

Hence, all of the objects listed above _are_ getting created. I can see how a few extra object allocations wouldn't matter _that_ much, but I also think it should be quite doable to not create a CacheableRequest if you're not going to use caching anyway. And yes, that could be a PR.

As for the use case: we're not exploiting anything. We essentially operate a proxy service for server-to-server traffic. It's a totally legitimate scenario and a cornerstone of our business at that. I'm also not sure why I need to defend my use case because all I did was point out that the default behavior (i.e., no options passed) is wasteful of resources. That even matters if you're making a single request, especially if it happens with the default configuration.

[szmarczak]

I can see how a few extra object allocations wouldn't matter that much

Yup, but it doesn't cache anything. It just initializes CacheableRequest, that's all.

We essentially operate a proxy service for server-to-server traffic.

I wouldn't use Node for that. Networking's slow as f...

I agree, it could be written better, we just need to figure out a way to do so. I tried optimalizing Got, but I ended up with a mess (performance increased but the code became unreadable)...

And yes, that could be a PR.

Great! If you got any ideas, feel free to share :)

[timdp]

I wouldn't use Node for that.

Little late for that. We've succesfully built a business around it. Got contributes to that, so definitely kudos to you guys, but also all of the above. :-)

As for juggling performance against maintainability: given all the async state, I think an RxJS-based HTTP client for Node would be an interesting experiment. However, that's more of a thought experiment and the grass is always greener on the other side.

[szmarczak]

Let's focus on optimizing Got then :) Two heads are better than one :)

[sindresorhus]

I think an RxJS-based HTTP client for Node would be an interesting experiment.

RxJS comes with its own problems.

[timdp]

That it does.