{{ message }}
perf(discovery): reduce per-file allocations in discovery phase#9991
Open
qoole wants to merge 2 commits into
Open
perf(discovery): reduce per-file allocations in discovery phase#9991qoole wants to merge 2 commits into
qoole wants to merge 2 commits into
Conversation
qoole
force-pushed
the
perf/discovery-allocations
branch
from
d94f294 to
e1cbaca
Compare
Contributor
mgallien
requested changes
May 22, 2026
mgallien
left a comment
mgallien
left a comment
Collaborator
There was a problem hiding this comment.
there are two types of changes in your PR
can you split them into separate commits with proper commit messages ?
they might even makes sense to be split into different PRs
qoole
force-pushed
the
perf/discovery-allocations
branch
from
e1cbaca to
e0b8acf
Compare
Contributor
Author
|
Split into two commits as requested:
Kept them in this PR for review locality — happy to split into separate PRs if you'd prefer. |
mgallien
requested changes
Jun 9, 2026
mgallien
left a comment
mgallien
left a comment
Collaborator
There was a problem hiding this comment.
we have a general disagreement in the team that switching from QTimer::singleShot(0 will have any impact
can you remove this commit and the other one will be ready to be merged ?
…g jobs Replace the 5 QTimer::singleShot(0, ...) calls that defer DiscoveryPhase::scheduleMoreJobs with QMetaObject::invokeMethod(Qt::QueuedConnection). Both achieve the same deferred (queued) invocation, but invokeMethod does not allocate a QTimer on the heap for each call. In a large sync with many directories this eliminates thousands of unnecessary heap allocations. Signed-off-by: Qoole <2862661+qoole@users.noreply.github.com>
Replace QString::split('.') with indexOf/lastIndexOf to extract the
basename and extension without allocating a QList<QString> per file,
and hold the slices as QStringView so the comparisons against
forbiddenFilenames / forbiddenBasenames / forbiddenExtensions run
against zero-copy views into the original QString instead of allocating
new QString copies.
Signed-off-by: Qoole <2862661+qoole@users.noreply.github.com>
mgallien
force-pushed
the
perf/discovery-allocations
branch
from
e0b8acf to
db670cb
Compare
Collaborator
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three micro-optimizations to the discovery hot path that fire once per file/directory in a sync. With large folder trees these allocations add up to noticeable GC and heap churn.
Changes
QString::split('.')allocates aQList<QString>plus copies for each segment, just to recover the basename and extension. Replaced withindexOf/lastIndexOfto find the cut points directly.QStringViewslices into the originalQStringrather than allocating new ownedQStringcopies. The downstream forbidden-filename / forbidden-basename / forbidden-extension comparisons all compare againstQStringView-compatible data, so no copies are needed.QTimer::singleShot(0, ...)call sites in discovery were replaced withQMetaObject::invokeMethod(Qt::QueuedConnection). Both schedule a slot for the next event loop iteration, but the latter does not allocate aQTimerobject on the heap. In a large sync with many directories this eliminates thousands ofQTimerallocations.Behavior is unchanged — same parsing results, same scheduling semantics.
Checklist
AI (if applicable)