perf: optimize `RequestList` memory footprint#2466

B4nan · 2024-05-14T16:08:42Z

The request list now delays the conversion of the source items into the Request objects, resulting in a significantly less memory footprint.

Related: https://apify.slack.com/archives/C0L33UM7Z/p1715109984834079

B4nan · 2024-05-14T16:26:45Z

~~The test suite is apparently stuck, but the tests on the platform look promising, so it's in the right direction.~~

Here is the run with 4 million items in RL with the patch:

https://console.apify.com/actors/Tcx7qJL5NP2PvmZ5c/runs/tuKGnab8BqrKlILHC#log

And the time to process the first request since Actor.init is now 11s:

2024-05-14T16:07:29.671Z INFO  System info {"apifyVersion":"3.2.0","apifyClientVersion":"2.9.3","crawleeVersion":"3.9.3-beta.42","osType":"Linux","nodeVersion":"v20.12.2"}
2024-05-14T16:07:40.741Z run with requests 4000001
2024-05-14T16:07:40.838Z INFO  CheerioCrawler: Starting the crawler.
2024-05-14T16:07:41.105Z INFO  CheerioCrawler: handling {"url":"http://www.example.com/?q=4"}

And here without the patch:

https://console.apify.com/actors/Tcx7qJL5NP2PvmZ5c/runs/qSuwhtHPzcS1N0WpN#log

The time to process the first request since Actor.init was ~85s:

2024-05-14T16:22:29.251Z INFO  System info {"apifyVersion":"3.2.0","apifyClientVersion":"2.9.3","crawleeVersion":"3.9.3-beta.42","osType":"Linux","nodeVersion":"v20.12.2"}
2024-05-14T16:23:54.410Z run with requests 4000001
2024-05-14T16:23:54.514Z INFO  CheerioCrawler: Starting the crawler.
2024-05-14T16:23:54.718Z INFO  CheerioCrawler: handling {"url":"http://www.example.com/?q=3"}

barjin · 2024-05-15T11:23:33Z

+    private ensureRequest(requestLike: Request | RequestOptions, index: number): Request {
+        if (requestLike instanceof Request) {
+            return requestLike;
+        }
+
+        this.requests[index] = new Request(requestLike);
+        return this.requests[index] as Request;
+    }


This is a bit unclear to me, why do we want to store the Request object back in the array?

Wouldn't something like

makeRequest(r: RequestLike) : Request { if(r instanceof Request) return r; return new Request(r); }

work just fine? Or do the (possible) repeated request constructions pose a performance problem?

because when we reclaim requests, we need to reclaim the same request object that was returned via fetchNextRequest (otherwise tests break, haven't been debugging why more in detail)

will add a comment for this

fetchNextRequest would still return a Request instance... the same one we would pass to reclaim later even.

But whatever, let's treat this as a hotfix, I assume the Delivery team depends on this. We'll revisit this in v4 either way, judging by the comments in Request?

the thing is, you need to persist the reclaimed requests, and that needs to happen with their metadata (e.g. userData), if we wouldn't do this, we would loose that completely on migration. at least i think so.

I see, sounds legit 👍🏽

B4nan · 2024-05-15T11:45:05Z

Last thing to deal with, migrations are broken:

2024-05-15T11:26:46.649Z ERROR CheerioCrawler: The crawler attempted to persist its request list's state and failed due to missing or invalid config. Make sure to use either RequestList.open() or the "stateKeyPrefix" option of RequestList constructor to ensure your crawling state is persisted through host migrations and restarts.

janbuchar · 2024-05-15T11:44:44Z

    }

-    protected _computeUniqueKey({ url, method, payload, keepUrlFragment, useExtendedUniqueKey }: ComputeUniqueKeyOptions) {
+    // TODO: only for better BC, remove in v4


Please make an issue to track that 🙂

we have other things in the code that are marked just with a TODO, since they are only about removing stuff (namely the old variants for crawler options like handlePageFunction). we can create one issue for all of such things later, for now i'd say a todo in the code is enough.

(but other than that I am on your side, i dislike TODOs in code very much, since they are always left forgotten, lol)

Well, there is a rule in Ruff (Python linter) that forces you to add issue links to TODO comments, and while it's a pain in the backside, it prevents forgotten TODOs 🙂

Maybe we could do something similar - it would even make it possible to estimate the difficulty of a "Remove v4 TODOs" issue.

janbuchar · 2024-05-15T11:50:10Z

            const name: string = null;
            const sources = [{ url: 'https://example.com' }];
-            const requests = sources.map(({ url }) => new Request({ url }));
+            const requests = sources.map(({ url }) => ({ url, uniqueKey: url }));


Are you changing this so that it passes the equality check? Also, it'd be nice to call that variable expectedRequests or something.

new Request creates the unique key in the constructor

B4nan · 2024-05-15T12:36:59Z

Looks like this is actually expected behaviour, the RL needs to be named or you need to provide persistStateKey explicitly to be able to persist its state. Checking with the delivery now to see if this is correct, as I wasn't expecting that really.

github-actions Bot assigned B4nan May 14, 2024

github-actions Bot added this to the 89th sprint - Tooling team milestone May 14, 2024

github-actions Bot added the t-tooling Issues with this label are in the ownership of the tooling team. label May 14, 2024

This comment was marked as spam.

Sign in to view

B4nan added the adhoc Ad-hoc unplanned task added during the sprint. label May 14, 2024

B4nan force-pushed the perf-request-list branch from 4a60ba3 to d10308b Compare May 14, 2024 17:33

B4nan changed the title ~~perf: try to optimize request list memory footprint~~ perf: optimize RequestList memory footprint May 14, 2024

B4nan force-pushed the perf-request-list branch from d10308b to 367962b Compare May 14, 2024 17:45

B4nan marked this pull request as ready for review May 14, 2024 17:45

github-actions Bot added the tested Temporary label used only programatically for some analytics. label May 14, 2024

B4nan force-pushed the perf-request-list branch 2 times, most recently from ce071e7 to 87c3e55 Compare May 14, 2024 17:56

B4nan requested a review from barjin May 14, 2024 18:11

perf: try to optimize request list memory footprint

8b32018

B4nan force-pushed the perf-request-list branch from 87c3e55 to 8b32018 Compare May 14, 2024 18:12

barjin reviewed May 15, 2024

View reviewed changes

barjin approved these changes May 15, 2024

View reviewed changes

janbuchar approved these changes May 15, 2024

View reviewed changes

B4nan merged commit 12210bd into master May 15, 2024

B4nan deleted the perf-request-list branch May 15, 2024 17:33

B4nan mentioned this pull request May 29, 2024

feat: Sitemap-based request list implementation #2498

Merged

2 tasks

barjin mentioned this pull request Nov 20, 2024

feat: Handle request list user input apify/apify-sdk-python#326

Merged

barjin mentioned this pull request May 19, 2025

fix(scrapers): Use RequestQueue directly to avoid excessive RQ writes on runs with large startUrls list apify/apify-sdk-js#393

Merged

Uh oh!

Conversation

B4nan commented May 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as spam.

Uh oh!

B4nan commented May 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

B4nan commented May 15, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

B4nan May 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

B4nan commented May 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

B4nan commented May 14, 2024 •

edited

Loading

B4nan commented May 14, 2024 •

edited

Loading

B4nan May 15, 2024 •

edited

Loading