Skip to content

Implement adaptive chunk sizes / adaptive speed benchmark results - #2427

Open
mr-creditcard wants to merge 17 commits into
hashtopolis:masterfrom
mr-creditcard:2295-adaptivechunks
Open

mr-creditcard wants to merge 17 commits into
hashtopolis:masterfrom
mr-creditcard:2295-adaptivechunks

Conversation

@mr-creditcard

@mr-creditcard mr-creditcard commented Aug 7, 2026

Copy link
Copy Markdown

This PR is a continuation of the work done by @0xVavaldi in #729. I have cherry-picked their commits and added my own on top in an attempt to keep attribution in place. It addresses issues #551 & #2295.

The aforementioned PR has a number of requirements set by @s3inlc in the comments which have remained unaddressed thusfar, this PR aims to implement these. The changes include:

Did some quick tests with this code and it seems to work fine. Would appreciate someone else testing this as well though!

Closes #551 & #2295.

@mr-creditcard
mr-creditcard force-pushed the 2295-adaptivechunks branch 3 times, most recently from 0bc5c8a to f9c6e4c Compare August 11, 2026 08:41
@mr-creditcard
mr-creditcard marked this pull request as ready for review August 11, 2026 08:42

@jessevz jessevz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks really good, apart from the small syntax error phpstan picked up and the postgres syntax it looks great. I have not tested the code yet but once these things are fixed I can do a test run. Maybe @s3inlc also want to review this

Comment thread src/migrations/postgres.1/20260810144313_chunk-auto-tune-config.sql Outdated
Comment thread src/inc/utils/TaskUtils.php Outdated
@mr-creditcard
mr-creditcard force-pushed the 2295-adaptivechunks branch 4 times, most recently from 370b6aa to 01d1b4d Compare September 8, 2026 12:07
@mr-creditcard

Copy link
Copy Markdown
Author

Rebased this PR on master!

@s3inlc s3inlc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for creating the PR, there is just some minor thing to adjust, otherwise this should be fine. Though we will have to adjust also in the frontend, so that the config variable can be changed there, so there may be some delay in merging it in here until the frontend also is ready with the change.

Comment thread src/inc/utils/TaskUtils.php
@four-random-common-words

Copy link
Copy Markdown
Contributor

@s3inlc Frontend PR for this: hashtopolis/web-ui#823

@s3inlc
s3inlc self-requested a review September 14, 2026 14:30

@s3inlc s3inlc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your adjustments, sorry I missed the <0 comparison in the first review, beside this I think it looks good to merge.
As part of the merge it is needed to adjust the migrations filenames to be adjusted to the current date and time (to keep script order for migrations in master). Can you just set it to an actual date and time together with the small change and then I'll merge shortly after.

Comment thread src/inc/utils/TaskUtils.php Outdated
@mr-creditcard

Copy link
Copy Markdown
Author

[...] As part of the merge it is needed to adjust the migrations filenames to be adjusted to the current date and time (to keep script order for migrations in master). Can you just set it to an actual date and time together with the small change and then I'll merge shortly after.

Did that just now!

@s3inlc

s3inlc commented Sep 15, 2026

Copy link
Copy Markdown
Member

Did that just now!

Perfect, thank you! Sorry that I didn't trigger the workflows earlier, there seems to be a syntax issue with the postgres migration (I think the single quotes are not valid, probably no quotes should be used on the column names). And also PHPstan highlighted something (should be $agent instead of $this->agent and probably wrapping intval() around the benchmark value should do the trick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Adaptive chunk sizes

5 participants