perf(logging): reduce logging overhead in hot paths#9990
Conversation
|
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.
thanks
see my inline comment
| qCDebug(lcDb) << "Updating file record for path:" << record.path() << "inode:" << record._inode | ||
| << "modtime:" << record._modtime << "type:" << record._type << "etag:" << record._etag | ||
| << "fileId:" << record._fileId << "remotePerm:" << record._remotePerm.toString() | ||
| << "fileSize:" << record._fileSize << "checksum:" << record._checksumHeader | ||
| << "e2eMangledName:" << record.e2eMangledName() << "isE2eEncrypted:" << record.isE2eEncrypted() | ||
| << "lock:" << (record._lockstate._locked ? "true" : "false") | ||
| << "lock owner type:" << record._lockstate._lockOwnerType | ||
| << "lock owner:" << record._lockstate._lockOwnerDisplayName | ||
| << "lock owner id:" << record._lockstate._lockOwnerId | ||
| << "lock editor:" << record._lockstate._lockEditorApp | ||
| << "sharedByMe:" << record._sharedByMe | ||
| << "isShared:" << record._isShared | ||
| << "lastShareStateFetchedTimestamp:" << record._lastShareStateFetchedTimestamp | ||
| << "isLivePhoto" << record._isLivePhoto | ||
| << "livePhotoFile" << record._livePhotoFile | ||
| << "folderQuota - bytesUsed:" << record._folderQuota.bytesUsed << "bytesAvailable:" << record._folderQuota.bytesAvailable; |
There was a problem hiding this comment.
we often need this log output when debugging an issue an user had in real life
this is the main reason for the info log level
can you revert this change ?
There was a problem hiding this comment.
Reverted in ceda7a5 — the qCInfo level on setFileRecord is restored. Kept the rest of the logging-overhead changes.
- Remove dead debug lambda in PUTFileJob::start() that constructed a temporary QLoggingCategory on every upload progress tick. The signal is already connected to the proper handler on the next line. - Add QtInfoMsg default to 3 Q_LOGGING_CATEGORY declarations that were missing it, preventing debug-level output from being active by default. - Remove tr() and QString::number() allocations from a qCDebug call in usermodel.cpp quota update path. Signed-off-by: Qoole <2862661+qoole@users.noreply.github.com>
qoole
force-pushed
the
perf/logging-hot-paths
branch
from
0f1cba1 to
ceda7a5
Compare
|
Addressed in ceda7a5: reverted the |
2 participants
Summary
Reduces logging overhead in several frequently-hit code paths:
PUTFileJob::start(): removes a dead debug lambda that constructed a temporaryQLoggingCategoryon every upload progress tick. The progress signal is already connected to the proper handler on the next line, so the lambda was a no-op that ran on every chunk.SyncJournalDb::setFileRecord: downgrades the 28-field log line fromqCInfotoqCDebugso it no longer fires in production on every file record update. With large syncs this generates a substantial amount of log spam at info level.Q_LOGGING_CATEGORYdefaults: addsQtInfoMsgdefault to three category declarations that were missing it, preventing debug-level output from being active by default in those modules.usermodel.cppquota update: removestr()andQString::number()allocations from aqCDebugcall that runs on every quota refresh, even when debug logging is not enabled.Checklist
AI (if applicable)