feat(editor): implement lazy loading for tab pages#452
Open
pengfeixx wants to merge 1 commit intolinuxdeepin:masterfrom
Open
feat(editor): implement lazy loading for tab pages#452pengfeixx wants to merge 1 commit intolinuxdeepin:masterfrom
pengfeixx wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pengfeixx The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Note
详情{
"src/widgets/window.cpp": [
{
"line": " QString key = \"base/enable\";",
"line_number": 389,
"rule": "S106",
"reason": "Var naming | 64f28539d9"
}
]
} |
Reviewer's GuideImplements lazy loading for editor tabs so only the active tab is fully instantiated at startup or when opening files, while non-focused tabs are represented as unloaded entries that are materialized on demand; also hardens many window actions against null wrappers/text editors and integrates unloaded tabs into backup/recovery/close flows to avoid crashes and ensure state persistence. Sequence diagram for lazy loading when opening files into an existing windowsequenceDiagram
actor User
participant StartManager
participant Window
participant Tabbar
participant EditWrapper
User->>StartManager: openFilesInTab(files)
loop for each canonicalFile in files
StartManager->>Window: addTabLazy(canonicalFile, fileName, canonicalFile)
Window->>Tabbar: addTab(filePath, tabName, truePath)
Note over Window,Tabbar: Tab button is created only
Window->>Window: m_unloadedTabs.insert(filePath)
end
Note over User,Tabbar: Later, user clicks on one of the newly opened tabs
User->>Tabbar: select tab(index)
Tabbar->>Window: handleCurrentChanged(index)
Window->>Tabbar: fileAt(index)
Tabbar-->>Window: filepath
alt tab is in m_unloadedTabs
Window->>Window: ensureTabLoaded(filepath)
Window-->>Window: check m_unloadedTabs.contains(filepath)
Window->>Window: m_unloadedTabs.remove(filepath)
Window->>Window: QFileInfo fi(filepath)
alt file missing or unsupported
Window->>Tabbar: closeTab(indexOf(filepath))
Window-->>Window: ensureTabLoaded returns false
Window->>Tabbar: count()
alt there are other tabs
Window->>Tabbar: setCurrentIndex(0)
end
else file exists and mime type supported
Window->>Window: createEditor()
Window-->>EditWrapper: wrapper
Window->>EditWrapper: openFile(filepath, truePath)
Window->>StartManager: findBookmark(truePath)
StartManager-->>Window: bookmarkInfo
Window->>EditWrapper: textEditor()
EditWrapper-->>Window: TextEditor
Window->>TextEditor: setBookMarkList(bookmarkInfo)
Window->>Window: m_wrappers[filepath] = wrapper
Window->>Window: showNewEditor(wrapper)
Window-->>Window: ensureTabLoaded returns true
end
end
Window->>Window: currentWrapper()
Window-->>Window: EditWrapper for filepath
Window->>EditWrapper: bottomBar()
EditWrapper-->>Window: BottomBar
Window->>BottomBar: show()
Window->>BottomBar: updateSize(BottomBar.defaultHeight(), false)
Sequence diagram for session recovery with lazy-loaded tabssequenceDiagram
participant StartManager
participant Window
participant Tabbar
participant EditWrapper
participant TextEditor
StartManager->>StartManager: recoverFile(window)
StartManager->>StartManager: parse m_qlistTemFile to JSON
loop first pass over recovery JSON
StartManager->>StartManager: validate file paths and metadata
StartManager->>StartManager: analyzeBookmakeInfo(bookMark)
StartManager->>StartManager: determine isFocusTab
StartManager->>StartManager: build RecoverEntry
StartManager->>StartManager: entries.append(entry)
end
loop second pass over entries
alt entry.isFocus is true
StartManager->>Window: addTemFileTab(entry.openPath, entry.tabName, entry.truePath, entry.lastModifiedTime, entry.isTemFile)
Window->>EditWrapper: wrapper(entry.openPath)
EditWrapper-->>Window: EditWrapper
alt entry.bookmarks not empty
Window->>EditWrapper: textEditor()
EditWrapper-->>Window: TextEditor
Window->>TextEditor: setBookMarkList(entry.bookmarks)
else global bookmark exists
StartManager->>StartManager: m_bookmarkTable.value(entry.truePath)
StartManager-->>Window: bookmarkList
Window->>EditWrapper: textEditor()
EditWrapper-->>Window: TextEditor
Window->>TextEditor: setBookMarkList(bookmarkList)
end
else non focus tab
StartManager->>Window: addTabLazy(entry.openPath, entry.tabName, entry.truePath)
Window->>Tabbar: addTab(filePath, tabName, truePath)
Window->>Window: m_unloadedTabs.insert(entry.openPath)
end
end
StartManager->>Window: set focus window and tab(focusPath)
Window->>Tabbar: setCurrentIndex(indexOf(focusPath))
Note over Window, Tabbar: Focus tab is fully loaded, others remain lazy until activated
Class diagram for lazy-loaded editor tabs and related componentsclassDiagram
class Window {
+QMap~QString, EditWrapper*~ m_wrappers
+QSet~QString~ m_unloadedTabs
+void addTab(QString filepath, bool activeTab)
+void addTabWithWrapper(EditWrapper* wrapper, QString filepath, QString truePath, QString tabName, int index)
+void addTabLazy(QString filePath, QString tabName, QString truePath)
+bool ensureTabLoaded(QString filePath)
+bool closeTab()
+bool closeTab(QString filePath)
+bool closeAllFiles()
+void backupFile()
+void handleCurrentChanged(int index)
+EditWrapper* currentWrapper()
+EditWrapper* wrapper(QString filePath)
}
class StartManager {
+QList~Window*~ m_windows
+QMap~QString, QList~int~~ m_bookmarkTable
+QStringList m_qlistTemFile
+int recoverFile(Window* window)
+void openFilesInTab(QStringList files)
+QList~int~ analyzeBookmakeInfo(QString info)
+static StartManager* instance()
}
class StartManager_RecoverEntry {
+Window* win
+QString openPath
+QString tabName
+QString truePath
+QString lastModifiedTime
+bool isTemFile
+bool isFocus
+bool hasTemFile
+QList~int~ bookmarks
}
class Tabbar {
+int count()
+QString currentPath()
+QString fileAt(int index)
+int indexOf(QString filePath)
+QString truePathAt(int index)
+void addTab(QString filePath, QString tabName, QString truePath)
+void closeTab(int index)
+void setCurrentIndex(int index)
+QPixmap createDragPixmapFromTab(int index, QStyleOptionTab option)
}
class EditWrapper {
+TextEditor* textEditor()
+BottomBar* bottomBar()
+bool getFileLoading()
+void openFile(QString filePath, QString truePath)
+void updateHighlighterAll()
+CSyntaxHighlighter* getSyntaxHighlighter()
+void checkForReload()
+void hideWarningNotices()
+void showNotify(QString message, bool warning)
+bool cloneLargeDocument(EditWrapper* wrapper)
}
class TextEditor {
+QString getFilePath()
+QString getTruePath()
+bool getReadOnlyMode()
+void setBookMarkList(QList~int~ list)
+QTextDocument* document()
+void print(DPrinter* printer)
+void setFocus()
+void setCursorStart(int position)
+void saveMarkStatus()
+void updateCursorKeywordSelection(QString keyword, bool state)
+void removeKeywords()
+void replaceAll(QString replaceText, QString withText)
+void replaceNext(QString replaceText, QString withText)
+void replaceRest(QString replaceText, QString withText)
+void beforeReplace(QString text)
+void tellFindBarClose()
+void setCodeFoldWidgetHide(bool hide)
+int getCurrentLine()
+int getCurrentColumn()
+int getScrollOffset()
+void setBookMarkList(QList~int~ bookmarkList)
+QPalette palette()
}
class BottomBar {
+static int defaultHeight()
+void updateSize(int height, bool animate)
+void show()
+bool isHidden()
+void setChildrenFocus(bool forward, QWidget* start)
}
class StartManager_Singleton {
+static StartManager* instance()
}
Window --> Tabbar : uses
Window --> EditWrapper : owns via m_wrappers
Window --> TextEditor : via EditWrapper
Window --> BottomBar : via EditWrapper
Window --> StartManager : uses singleton
Window --> StartManager_RecoverEntry : used indirectly via StartManager
StartManager --> Window : manages
StartManager --> StartManager_RecoverEntry : aggregates
StartManager_RecoverEntry --> Window
StartManager_RecoverEntry --> TextEditor : bookmarks
Tabbar --> Window : casts parent window
Tabbar --> EditWrapper : via Window.wrapper
EditWrapper --> TextEditor
EditWrapper --> BottomBar
StartManager_Singleton <|-- StartManager
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
StartManager::recoverFile, non-focus draft/unsupported files that previously usedaddTemFileTabare now added viaaddTabLazy, which later enforcesUtils::isMimeTypeSupportinensureTabLoadedand will close those tabs on activation; consider preserving the old behavior for drafts/unsupported MIME types so they still restore correctly when not focused. - In
Tabbar::createDragPixmapFromTab, returning an emptyQPixmapwhenwrapperis null will disable drag for unloaded tabs; if drag-and-drop should be supported for lazy-loaded tabs, consider generating the pixmap from tab text/icon instead of bailing out when the editor is not yet created.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `StartManager::recoverFile`, non-focus draft/unsupported files that previously used `addTemFileTab` are now added via `addTabLazy`, which later enforces `Utils::isMimeTypeSupport` in `ensureTabLoaded` and will close those tabs on activation; consider preserving the old behavior for drafts/unsupported MIME types so they still restore correctly when not focused.
- In `Tabbar::createDragPixmapFromTab`, returning an empty `QPixmap` when `wrapper` is null will disable drag for unloaded tabs; if drag-and-drop should be supported for lazy-loaded tabs, consider generating the pixmap from tab text/icon instead of bailing out when the editor is not yet created.
## Individual Comments
### Comment 1
<location path="src/widgets/window.cpp" line_range="841-844" />
<code_context>
{
qDebug() << "Enter closeTab with path:" << filePath;
+
+ if (m_unloadedTabs.contains(filePath)) {
+ qDebug() << "Closing unloaded tab:" << filePath;
+ m_unloadedTabs.remove(filePath);
+ m_tabbar->closeTab(m_tabbar->indexOf(filePath));
+ if (m_wrappers.isEmpty() && m_unloadedTabs.isEmpty()) {
+ close();
</code_context>
<issue_to_address>
**issue (bug_risk):** Validate tab index before closing unloaded tab
In `closeTab(const QString &filePath)`, for unloaded tabs you pass `m_tabbar->indexOf(filePath)` directly into `closeTab()`. If `filePath` is not found (e.g., out-of-sync state or race with tab removal), `indexOf` returns `-1`, which is unsafe. Please store the result in a local variable, verify it is `>= 0`, and only then call `closeTab(idx)`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+841
to
+844
| if (m_unloadedTabs.contains(filePath)) { | ||
| qDebug() << "Closing unloaded tab:" << filePath; | ||
| m_unloadedTabs.remove(filePath); | ||
| m_tabbar->closeTab(m_tabbar->indexOf(filePath)); |
There was a problem hiding this comment.
issue (bug_risk): Validate tab index before closing unloaded tab
In closeTab(const QString &filePath), for unloaded tabs you pass m_tabbar->indexOf(filePath) directly into closeTab(). If filePath is not found (e.g., out-of-sync state or race with tab removal), indexOf returns -1, which is unsafe. Please store the result in a local variable, verify it is >= 0, and only then call closeTab(idx).
pengfeixx
force-pushed
the
feat/lazy-load-tab
branch
2 times, most recently
from
58b0d22 to
c33095c
Compare
|
Note
详情{
"src/widgets/window.cpp": [
{
"line": " QString key = \"base/enable\";",
"line_number": 389,
"rule": "S106",
"reason": "Var naming | 64f28539d9"
}
]
} |
pengfeixx
force-pushed
the
feat/lazy-load-tab
branch
2 times, most recently
from
58b0d22 to
89937d3
Compare
|
Note
详情{
"src/widgets/window.cpp": [
{
"line": " QString key = \"base/enable\";",
"line_number": 389,
"rule": "S106",
"reason": "Var naming | 64f28539d9"
}
]
} |
Add lazy loading mechanism to improve startup performance. Only the focused tab is fully loaded during startup, other tabs are loaded on demand when user switches to them. 实现标签页延迟加载机制以优化启动性能。启动时仅加载焦点标签， 其他标签在用户切换时按需加载。 Log: 实现标签页延迟加载功能 PMS: BUG-348319 Influence: 优化编辑器启动性能，减少启动时的文件I/O和内存占用。同时修复延迟 加载场景下的空指针崩溃问题，提升稳定性。
pengfeixx
force-pushed
the
feat/lazy-load-tab
branch
from
89937d3 to
8e4a679
Compare
|
Note
详情{
"src/widgets/window.cpp": [
{
"line": " QString key = \"base/enable\";",
"line_number": 389,
"rule": "S106",
"reason": "Var naming | 64f28539d9"
}
]
} |
deepin pr auto review这段代码实现了一个标签页延迟加载（Lazy Loading）的功能，主要目的是在打开多个文件时，只加载当前活动的标签页内容，其他标签页仅显示标签按钮，直到被激活时才加载内容。这种优化可以显著减少内存占用和启动时间，特别是在处理大量文件时。 以下是对代码的详细审查和改进建议： 1. 语法逻辑审查优点：
改进建议：
2. 代码质量审查优点：
改进建议：
3. 代码性能审查优点：
改进建议：
4. 代码安全审查优点：
改进建议：
5. 其他建议
总结这段代码整体质量较高，实现了标签页延迟加载的功能，有效优化了内存使用和启动时间。主要的改进方向是进一步细化函数、减少重复代码、增强错误处理和状态保存的完整性，以及考虑线程安全和用户体验方面的优化。 |
Contributor
|
TAG Bot New tag: 6.5.49 |
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.
Add lazy loading mechanism to improve startup performance. Only the focused tab is fully loaded during startup, other tabs are loaded on demand when user switches to them.
实现标签页延迟加载机制以优化启动性能。启动时仅加载焦点标签，
其他标签在用户切换时按需加载。
Log: 实现标签页延迟加载功能
PMS: BUG-348319
Influence: 优化编辑器启动性能，减少启动时的文件I/O和内存占用。同时修复延迟
加载场景下的空指针崩溃问题，提升稳定性。
Summary by Sourcery
Introduce lazy-loading support for editor tabs and make tab-related operations robust to unloaded editors.
New Features:
Bug Fixes:
Enhancements: