feat: add dde session boot time analysis tool#202
feat: add dde session boot time analysis tool#202yixinshark merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's GuideAdds a new Bash utility script to analyze DDE user-session boot performance, computing relative timelines from systemd user service timestamps, filtering and presenting systemd-analyze blame data, listing key user services/processes with memory and start times, inspecting journal logs for dde-shell/desktop bottlenecks, validating systemd service unit configs, and optionally generating a Markdown report with findings and optimization suggestions. Sequence diagram for running the DDE boot time analysis and optional report generationsequenceDiagram
actor User
participant Script_analyze_boot_time_sh as Script_analyze_boot_time_sh
participant SystemdUser as systemd_user
participant Journalctl as journalctl
participant ProcFS as procfs
participant FS as filesystem
User->>Script_analyze_boot_time_sh: run ./analyze-boot-time.sh [--report]
Script_analyze_boot_time_sh->>SystemdUser: show dde-session-* targets and services
SystemdUser-->>Script_analyze_boot_time_sh: timestamps and unit properties
Script_analyze_boot_time_sh->>Script_analyze_boot_time_sh: compute baseline and relative times
Script_analyze_boot_time_sh->>SystemdUser: systemd-analyze --user blame
SystemdUser-->>Script_analyze_boot_time_sh: blame list
Script_analyze_boot_time_sh->>SystemdUser: show Type for each blamed service
SystemdUser-->>Script_analyze_boot_time_sh: Type=notify/dbus
Script_analyze_boot_time_sh->>SystemdUser: list-units --type=service --state=running
SystemdUser-->>Script_analyze_boot_time_sh: running service list
Script_analyze_boot_time_sh->>ProcFS: read /proc/PID/status and cmdline
ProcFS-->>Script_analyze_boot_time_sh: memory, ppid, command line
Script_analyze_boot_time_sh->>Journalctl: journalctl --user -u dde-shell@DDE.service
Journalctl-->>Script_analyze_boot_time_sh: dde-shell logs
Script_analyze_boot_time_sh->>Journalctl: journalctl --user -u dde-shell-plugin@org.deepin.ds.desktop.service
Journalctl-->>Script_analyze_boot_time_sh: desktop plugin logs
Script_analyze_boot_time_sh->>FS: read unit files under /usr/lib/systemd/user
FS-->>Script_analyze_boot_time_sh: unit contents
Script_analyze_boot_time_sh->>Script_analyze_boot_time_sh: derive optimization suggestions
alt --report flag provided
Script_analyze_boot_time_sh->>FS: write boot-analysis-YYYYMMDD-HHMMSS.md
FS-->>Script_analyze_boot_time_sh: report file path
Script_analyze_boot_time_sh-->>User: console summary + report path
else no --report
Script_analyze_boot_time_sh-->>User: console summary only
end
Flow diagram for DDE boot time analysis script main logicflowchart TD
Start[Start analyze-boot-time.sh] --> ParseArgs[Parse CLI args]
ParseArgs --> InitOutput[Print header and colorized sections]
InitOutput --> CollectTimestamps[Collect systemd user target timestamps]
CollectTimestamps --> FindBaseline[Find earliest pre-stage service as baseline]
FindBaseline -->|baseline found| CalcPhaseTimes[Compute relative times for targets]
FindBaseline -->|no baseline| ErrorExit[Print error and exit]
CalcPhaseTimes --> Phase1Table[Print phase tables for pre/core/initialized/session]
Phase1Table --> BlameSection[Run systemd-analyze blame and filter Type=notify/dbus]
BlameSection --> ProcList[List top 50 running user services with PID, mem, start time]
ProcList --> LogAnalysis[Analyze dde-shell and desktop plugin logs for bottlenecks]
LogAnalysis --> UnitValidation[Inspect installed user unit files for bad dependencies]
UnitValidation --> BuildSuggestions[Build optimization suggestions array]
BuildSuggestions --> Summary[Print timing summary and quality assessment]
Summary --> CheckReportFlag{--report passed?}
CheckReportFlag -->|yes| GenReport[Generate Markdown report]
CheckReportFlag -->|no| End[End script]
GenReport --> End
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The function
get_svc_timestampsis defined twice with identical content; consider keeping a single definition to avoid confusion and maintenance issues. - In
generate_markdown_report, variablesXSETTINGS_BEFOREandDAEMON_BEFOREare used but never assigned in the script, so the dependency check in the report is effectively broken—populate these values (e.g., from the earlier unit file inspection) or adjust the logic. - When building the service list with
systemctl --user list-units --type=service --state=running | awk '{print $1}', consider adding--no-legend --no-pageror otherwise filtering out headers to avoid treating non-unit lines as service names.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The function `get_svc_timestamps` is defined twice with identical content; consider keeping a single definition to avoid confusion and maintenance issues.
- In `generate_markdown_report`, variables `XSETTINGS_BEFORE` and `DAEMON_BEFORE` are used but never assigned in the script, so the dependency check in the report is effectively broken—populate these values (e.g., from the earlier unit file inspection) or adjust the logic.
- When building the service list with `systemctl --user list-units --type=service --state=running | awk '{print $1}'`, consider adding `--no-legend --no-pager` or otherwise filtering out headers to avoid treating non-unit lines as service names.
## Individual Comments
### Comment 1
<location path="tools/analyze-boot-time.sh" line_range="85-86" />
<code_context>
+ fi
+}
+
+# 辅助函数:获取服务的时间戳
+get_svc_timestamps() {
+ local svc="$1"
+ local start=$(get_timestamp "$svc" "ExecMainStartTimestampMonotonic")
</code_context>
<issue_to_address>
**suggestion:** get_svc_timestamps is defined twice; consider keeping a single definition to avoid divergence later.
This function is already defined earlier in the script with the same body. Duplicating it increases the risk that only one copy is updated later. Please keep a single definition and reuse it where needed.
Suggested implementation:
```
# Target 只有就绪时刻
PRE_READY=$(get_timestamp "dde-session-pre.target" "ActiveEnterTimestampMonotonic")
CORE_READY=$(get_timestamp "dde-session-core.target" "ActiveEnterTimestampMonotonic")
INIT_READY=$(get_timestamp "dde-session-initialized.target" "ActiveEnterTimestampMonotonic")
SESSION_READY=$(get_timestamp "dde-session.target" "ActiveEnterTimestampMonotonic")
```
This assumes the earlier definition of `get_svc_timestamps` (with the same body) remains in the script. If that earlier definition is not present or differs, keep a single definition near the top-level helpers section and remove any other copies similarly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # 辅助函数:获取服务的时间戳 | ||
| get_svc_timestamps() { |
There was a problem hiding this comment.
suggestion: get_svc_timestamps is defined twice; consider keeping a single definition to avoid divergence later.
This function is already defined earlier in the script with the same body. Duplicating it increases the risk that only one copy is updated later. Please keep a single definition and reuse it where needed.
Suggested implementation:
# Target 只有就绪时刻
PRE_READY=$(get_timestamp "dde-session-pre.target" "ActiveEnterTimestampMonotonic")
CORE_READY=$(get_timestamp "dde-session-core.target" "ActiveEnterTimestampMonotonic")
INIT_READY=$(get_timestamp "dde-session-initialized.target" "ActiveEnterTimestampMonotonic")
SESSION_READY=$(get_timestamp "dde-session.target" "ActiveEnterTimestampMonotonic")
This assumes the earlier definition of get_svc_timestamps (with the same body) remains in the script. If that earlier definition is not present or differs, keep a single definition near the top-level helpers section and remove any other copies similarly.
Add a professional boot time analysis script for DDE session. - Displays systemd user service timelines and critical paths. - Provides reliable systemd-analyze blame data by filtering notification/dbus types. - Lists session processes with memory usage and relative start times. - Analyzes dde-shell and desktop logs for internal bottlenecks. - Validates systemd service configurations and gives optimization advice. 新增 DDE 会话启动耗时及内存分析专业脚本。 - 显示 systemd 用户服务启动时间线和关键路径耗时。 - 通过过滤 notification/dbus 类型提供可靠的 systemd-analyze blame 数据。 - 列出会话进程及其内存占用和相对启动时刻。 - 分析 dde-shell 和桌面日志以识别内部瓶颈。 - 验证 systemd 服务配置并提供具体的优化建议。 Log: add dde session boot time analysis tool Pms: TASK-384099 Change-Id: Ib6fa8374eacc3c93822c1c17b3301386d42c92d9
4f3806e to
1f0928c
Compare
deepin pr auto review这份代码包含两个用于分析 DDE(Deepin Desktop Environment)启动性能和内存占用的 Bash 脚本。整体来看,代码结构清晰,功能完善,注释详细。以下是对代码的详细审查和改进建议: 一、语法和逻辑审查
二、代码质量改进
三、性能优化
四、安全性改进
五、具体改进建议
六、改进后的代码示例以下是改进后的 calc_ms() {
local start="$1"
local end="$2"
# 验证输入是否为数字
if ! [[ "$start" =~ ^[0-9]+$ ]] || ! [[ "$end" =~ ^[0-9]+$ ]]; then
echo "N/A"
return
fi
if [ "$start" = "0" ] || [ "$end" = "0" ] || [ -z "$start" ] || [ -z "$end" ]; then
echo "N/A"
return
fi
echo $(( (end - start) / 1000 ))
}七、总结这两个脚本整体质量较高,功能完善,但在错误处理、代码复用、性能优化和安全性方面还有改进空间。建议按照上述建议进行优化,以提高脚本的健壮性和可维护性。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mhduiy, yixinshark 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 |
Add a professional boot time analysis script for DDE session.
新增 DDE 会话启动耗时分析专业脚本。
Log: add dde session boot time analysis tool
Pms: TASK-384099
Change-Id: Ib6fa8374eacc3c93822c1c17b3301386d42c92d9
Summary by Sourcery
Add a DDE session boot time analysis script that inspects systemd user targets, session processes, logs, and service configs to highlight boot bottlenecks and optimization opportunities.
New Features: