Open
Conversation
meshExtractorにストップウォッチ機能を追加 同上
ウォークスルーメッシュ抽出機能に対してロギング機能を追加します。C++ヘッダーに新たなオーバーロード関数を導入し、C APIラッパーとC#ラッパーを拡張してロギングコールバックをサポートします。実装内ではStopwatch ユーティリティでタイミング計測を行い、主要な処理段階を診断ログに記録します。 変更箇所
シーケンス図sequenceDiagram
participant CSharp as C# Client
participant CWrapper as C Wrapper
participant CPP as C++ MeshExtractor
participant Logger as CityGMLLogger
CSharp->>CWrapper: ExtractInExtents(model, options, extents, logCallbacks, logLevel)
CWrapper->>CWrapper: Create PlateauDllLogger
CWrapper->>CWrapper: Configure callbacks
CWrapper->>CPP: extractInExtents(..., logger)
CPP->>Logger: Initialize Stopwatch
CPP->>Logger: Log "Initialize" stage
CPP->>Logger: Log "LOD loop" stage
CPP->>Logger: Log "Node arrange" stage
CPP->>Logger: Log "Texture packing" stage
CPP->>Logger: Log "Map attach" stage
CPP->>CWrapper: Return Model
CWrapper->>CSharp: Return APIResult
見積もりコードレビュー工数🎯 3 (中程度) | ⏱️ 約20分 複数レイヤ(C++、Cラッパー、C#)にまたがる一貫性のあるロギング機能追加であり、新規メソッドシグネチャ、タイミング計測ロジック、エラーハンドリングの検証が必要です。変更パターンは反復的でホモジーニアスですが、ロギング統合とタイミング機能については各ファイルで個別の確認が必要です。 関連する可能性のあるPR
ウサギからのお祝い詩
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 (2)
include/plateau/polygon_mesh/mesh_extractor.h (2)
37-37: 新しいオーバーロードに対してドキュメントコメントの追加を推奨します。logger パラメータの目的と使用方法を説明するドキュメントコメントがありません。既存のオーバーロードと同様に、関数の説明を追加することをお勧めします。
48-48: 新しいオーバーロードに対してドキュメントコメントの追加を推奨します。logger パラメータの目的と使用方法を説明するドキュメントコメントがありません。特に、nullptr を渡した場合のデフォルト動作について記載すると良いでしょう。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
include/plateau/polygon_mesh/mesh_extractor.h(3 hunks)src/c_wrapper/mesh_extractor_c.cpp(2 hunks)src/polygon_mesh/mesh_extractor.cpp(6 hunks)wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/PolygonMesh/MeshExtractor.cs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
include/plateau/polygon_mesh/mesh_extractor.h (1)
src/polygon_mesh/mesh_extractor.cpp (12)
extract(213-218)extract(213-214)extract(220-223)extract(220-221)extract(225-227)extract(225-225)extractInExtents(229-236)extractInExtents(229-231)extractInExtents(238-244)extractInExtents(238-241)extractInExtents(246-248)extractInExtents(246-246)
wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/PolygonMesh/MeshExtractor.cs (4)
wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/Native/NativeVectorExtent.cs (11)
Extent(21-27)NativeVectorExtent(8-80)NativeVectorExtent(10-12)NativeVectorExtent(14-19)Add(40-45)NativeMethods(54-79)DllImport(56-58)DllImport(60-62)DllImport(64-68)DllImport(70-73)DllImport(75-78)wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/Native/DllLogger.cs (3)
LogCallbacks(23-62)LogCallbacks(39-44)IntPtr(58-61)src/c_wrapper/mesh_extractor_c.cpp (4)
plateau_mesh_extractor_extract_in_extents_with_log(48-56)APIResult(17-24)APIResult(35-43)APIResult(48-62)wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/Interop/DLLUtil.cs (3)
DLLUtil(25-363)IntPtr(53-58)IntPtr(146-164)
src/c_wrapper/mesh_extractor_c.cpp (1)
src/polygon_mesh/mesh_extractor.cpp (6)
extractInExtents(229-236)extractInExtents(229-231)extractInExtents(238-244)extractInExtents(238-241)extractInExtents(246-248)extractInExtents(246-246)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: upload-dlls (macos-14, arm64)
- GitHub Check: upload-dlls (macos-14, x86_64)
- GitHub Check: upload-dlls (ubuntu-24.04, x86_64)
- GitHub Check: upload-dlls (windows-2022, x86_64)
- GitHub Check: build-and-test (macos-14, arm64)
- GitHub Check: build-and-test (ubuntu-24.04, x86_64)
- GitHub Check: build-and-test (windows-2022, x86_64)
🔇 Additional comments (6)
wrappers/csharp/LibPLATEAU.NET/CSharpPLATEAU/PolygonMesh/MeshExtractor.cs (1)
35-51: 実装は正しいですが、API の一貫性を確認してください。
ExtractInExtentsメソッドにはログ機能が追加されていますが、Extractメソッド（21行目）にはログ機能がありません。これは意図的な設計ですか？もしExtractが頻繁に使用される場合は、同様のログ機能を追加することを検討してください。src/c_wrapper/mesh_extractor_c.cpp (1)
48-65: 実装は正しいですが、API の一貫性について確認してください。
plateau_mesh_extractor_extract_in_extents_with_log関数は追加されていますが、対応するplateau_mesh_extractor_extract_with_log関数がありません。C# ラッパーでも同様の非対称性が見られます。これが意図的な設計である場合は問題ありませんが、将来的にextractにもログ機能が必要になる可能性を考慮してください。src/polygon_mesh/mesh_extractor.cpp (4)
73-82: 適切なロガーの初期化とエラーログ記録です。nullptr チェックによるデフォルトロガーの作成と、例外を投げる前のエラーログ記録は良い実装です。
88-208: 各処理段階での時間計測が適切に実装されています。Initialize、LOD loop、Node arrange、Texture packing、Map attach の各段階で時間計測とログ出力が行われており、パフォーマンス分析に有用です。PR の目的である「各段階の処理時間をログに出す」が正しく実装されています。
225-227: ロガー対応のオーバーロードが正しく実装されています。既存の API との互換性を保ちながら、ロガーパラメータを受け取る新しいオーバーロードが追加されています。
246-248: ロガー対応のオーバーロードが正しく実装されています。extractInExtents についても、既存の API との互換性を保ちながら、ロガーパラメータを受け取る新しいオーバーロードが追加されています。
Comment on lines
+23
to
+49
| class Stopwatch { | ||
| public: | ||
| Stopwatch(std::shared_ptr<citygml::CityGMLLogger> logger, const std::string& task_name) | ||
| : logger_(std::move(logger)), task_name_(task_name) { | ||
| total_start_time_ = std::chrono::steady_clock::now(); | ||
| stage_start_time_ = total_start_time_; | ||
| logger_->log(citygml::CityGMLLogger::LOGLEVEL::LL_INFO, task_name_ + " start."); | ||
| } | ||
|
|
||
| ~Stopwatch() { | ||
| const auto total_duration = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - total_start_time_).count(); | ||
| logger_->log(citygml::CityGMLLogger::LOGLEVEL::LL_INFO, task_name_ + " finished. Total: " + std::to_string(total_duration) + "ms"); | ||
| } | ||
|
|
||
| void log_stage(const std::string& stage_name) { | ||
| const auto end_time = std::chrono::steady_clock::now(); | ||
| const auto duration = std::chrono::duration_cast<std::chrono::milliseconds>(end_time - stage_start_time_).count(); | ||
| logger_->log(citygml::CityGMLLogger::LOGLEVEL::LL_INFO, " " + stage_name + " : " + std::to_string(duration) + "ms"); | ||
| stage_start_time_ = end_time; | ||
| } | ||
|
|
||
| private: | ||
| std::shared_ptr<citygml::CityGMLLogger> logger_; | ||
| std::string task_name_; | ||
| std::chrono::steady_clock::time_point total_start_time_; | ||
| std::chrono::steady_clock::time_point stage_start_time_; | ||
| }; |
There was a problem hiding this comment.
Stopwatch のデストラクタでの例外安全性に注意が必要です。
デストラクタ内で logger_->log() を呼び出していますが、スタックアンワインド中にこのデストラクタが呼ばれた場合、log メソッドが例外を投げると std::terminate() が呼ばれてしまいます。
以下のように noexcept 指定と try-catch で保護することを推奨します：
- ~Stopwatch() {
+ ~Stopwatch() noexcept {
+ try {
const auto total_duration = std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - total_start_time_).count();
logger_->log(citygml::CityGMLLogger::LOGLEVEL::LL_INFO, task_name_ + " finished. Total: " + std::to_string(total_duration) + "ms");
+ } catch (...) {
+ // ログ出力に失敗しても無視する
+ }
}🤖 Prompt for AI Agents
In src/polygon_mesh/mesh_extractor.cpp around lines 23 to 49, the Stopwatch
destructor calls logger_->log() which can throw during stack unwinding and cause
std::terminate; make the destructor noexcept and wrap its logging calls in a
try-catch block (catch all exceptions) while optionally checking logger_ for
null before use; do the same protection for any final logging in the destructor,
and consider using non-throwing logging helpers or std::terminate-safe fallback
if logging fails.
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
1 participant
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.
✅ レビュー前確認項目
✅ マージ前確認項目
🚀 実装内容
🌐 影響範囲
🛠️ 動作確認
unity sdkのfeat/import_timeブランチで処理時間がログで出ているのを確認できます
Summary by CodeRabbit
リリースノート