Cache boundQuery as lazy property to avoid redundant SQL parsing#20
Open
39ff wants to merge 1 commit into
Open
Cache boundQuery as lazy property to avoid redundant SQL parsing#2039ff wants to merge 1 commit into
39ff wants to merge 1 commit into
Conversation
The boundQuery getter was recomputed on every access (used from applyFilters and again via shortSql in getValueAt). Convert it from a computed property to a lazy-delegated property so the parsed/bound SQL is computed once per QueryEntry instance. The lazy val lives in the data class body so equals/hashCode/copy remain unaffected. https://claude.ai/code/session_01YAhwF4pT5iPQoSqA9iSEz7
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
WalkthroughQueryEntry.ktのboundQueryプロパティが従来のカスタムgetterから遅延初期化プロパティに変更されました。パラメータ置換ロジックは構造化され、明示的なcontinueステートメントが追加されましたが、機能は変わりません。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt (1)
44-44:LazyThreadSafetyMode.NONEによるロックオーバーヘッドの削減（任意）デフォルトの
by lazyはLazyThreadSafetyMode.SYNCHRONIZEDを使用し、初回アクセス時にロックを取得します。QueryEntryが常に EDT（Event Dispatch Thread）などの単一スレッドからのみアクセスされる場合は、by lazy(LazyThreadSafetyMode.NONE)に変更することで同期コストを省けます。- val boundQuery: String? by lazy { + val boundQuery: String? by lazy(LazyThreadSafetyMode.NONE) {バックグラウンドスレッドからアクセスされる可能性がある場合はデフォルトのままにしてください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt` at line 44, The boundQuery lazy property currently uses the default thread-safe lazy which incurs synchronization overhead; change the declaration of the boundQuery property in QueryEntry (the val boundQuery: String? by lazy { ... } property) to use by lazy(LazyThreadSafetyMode.NONE) { ... } to remove locking if QueryEntry instances are only accessed from a single thread (e.g., EDT); if QueryEntry may be accessed from background threads, keep the default synchronized lazy instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt`:
- Around line 55-66: The logic in QueryEntry.kt that parses line comments is
only searching for '\n' (eol = query.indexOf('\n', i)), so a lone '\r' ends up
treated as "no eol" and the rest of the query is appended incorrectly; update
the comment-handling code (the block referencing sb, query, i, and eol for '--'
and the analogous '#' block) to locate the nearest line terminator by searching
for both '\n' and '\r' and taking the earliest non-negative index (handling -1s
appropriately), then use that index to append until eol+1 (or to end if none
found) so '\r'‑only line endings are handled correctly; apply the same change to
the '#' comment handling block.
---
Nitpick comments:
In
`@jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt`:
- Line 44: The boundQuery lazy property currently uses the default thread-safe
lazy which incurs synchronization overhead; change the declaration of the
boundQuery property in QueryEntry (the val boundQuery: String? by lazy { ... }
property) to use by lazy(LazyThreadSafetyMode.NONE) { ... } to remove locking if
QueryEntry instances are only accessed from a single thread (e.g., EDT); if
QueryEntry may be accessed from background threads, keep the default
synchronized lazy instead.
Comment on lines
+55
to
66
| if (ch == '-' && i + 1 < query.length && query[i + 1] == '-' | ||
| && i + 2 < query.length && (query[i + 2] == ' ' || query[i + 2] == '\t' || query[i + 2] == '\n' || query[i + 2] == '\r')) { | ||
| val eol = query.indexOf('\n', i) | ||
| if (eol == -1) { | ||
| sb.append(query, i, query.length) | ||
| i = query.length | ||
| } else { | ||
| sb.append(query, i, eol + 1) | ||
| i = eol + 1 | ||
| } | ||
| continue | ||
| } |
There was a problem hiding this comment.
\r のみの行末記号で -- コメントの残りクエリ全体が誤って消費される
eol の検索に query.indexOf('\n', i) のみを使用しており、\r だけの行末（旧 Mac スタイル）の場合は eol == -1 となって、クエリの残り全体がコメントとして StringBuilder に追記されます。
例: "SELECT --\r WHERE id=?" → eol = -1 → sb.append(query, i, query.length) → プレースホルダ ? が置換されずに残る。
同じ問題は # コメント（lines 69–79）にも存在します。実際の SQL では \r 単体の行末は稀ですが、念のため \r の検索も追加することを推奨します。
🐛 修正案（`--` コメントおよび `#` コメントの両方）
- val eol = query.indexOf('\n', i)
+ var eol = query.indexOf('\n', i)
+ if (eol == -1) eol = query.indexOf('\r', i)
if (eol == -1) {
sb.append(query, i, query.length)
i = query.length
} else {
- sb.append(query, i, eol + 1)
- i = eol + 1
+ // \r\n の場合は \r も含めて読み飛ばす
+ val end = if (eol + 1 < query.length && query[eol] == '\r' && query[eol + 1] == '\n') eol + 2 else eol + 1
+ sb.append(query, i, end)
+ i = end
}# コメント（lines 70–77）も同様に修正してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@jetbrains-plugin/src/main/kotlin/com/mariadbprofiler/plugin/model/QueryEntry.kt`
around lines 55 - 66, The logic in QueryEntry.kt that parses line comments is
only searching for '\n' (eol = query.indexOf('\n', i)), so a lone '\r' ends up
treated as "no eol" and the rest of the query is appended incorrectly; update
the comment-handling code (the block referencing sb, query, i, and eol for '--'
and the analogous '#' block) to locate the nearest line terminator by searching
for both '\n' and '\r' and taking the earliest non-negative index (handling -1s
appropriately), then use that index to append until eol+1 (or to end if none
found) so '\r'‑only line endings are handled correctly; apply the same change to
the '#' comment handling block.
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.
The boundQuery getter was recomputed on every access (used from applyFilters and again via shortSql in getValueAt). Convert it from a computed property to a lazy-delegated property so the parsed/bound SQL is computed once per QueryEntry instance. The lazy val lives in the data class body so equals/hashCode/copy remain unaffected.
https://claude.ai/code/session_01YAhwF4pT5iPQoSqA9iSEz7
Summary by CodeRabbit
リリースノート