perf(discovery): reduce per-file allocations in discovery phase#9991
perf(discovery): reduce per-file allocations in discovery phase#9991qoole wants to merge 2 commits into
Conversation
qoole
force-pushed
the
perf/discovery-allocations
branch
from
d94f294 to
e1cbaca
Compare
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
mgallien
left a comment
mgallien
left a comment
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
…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>
qoole
force-pushed
the
perf/discovery-allocations
branch
from
e1cbaca to
e0b8acf
Compare
|
Split into two commits as requested:
Kept them in this PR for review locality — happy to split into separate PRs if you'd prefer. |
2 participants
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)