「ログ出力時に毎回ファイルを開いている」の修正 #67

Takym · 2021-03-19T22:38:36Z

Issue 番号 / Issue ID

目的 / Purpose

動作効率を改善させる為、ログ出力時に毎回ログファイルを開かない様にしました。

破壊的変更をもたらしますか / Does this introduce a breaking change?

[x] Yes
[ ] No

Pull Request の種類 / Pull Request type

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

検証方法 / How to test

コードの入手 / Get the code

git clone https://github.com/Takym/cocoa.git
cd cocoa
git checkout logging
dotnet restore

コードの検証 / Test the code

以前と同じ様にログ出力が行われるか検証してください。宜しくお願い致します。

確認事項 / What to check

LoggerService に記述されていたログ出力処理を LogWriter に移動しました。
- これに伴い LoggerService のコンストラクタの引数を変更しました。
極力仕様を変更しない様、FileStream には FileShare.ReadWrite を指定しています。
- 毎回ログファイルを開き直す事を前提としているプログラムも正しく動作する筈です。
LogWriter はファイル名の変更(日付の変更)を検出すると自動的にファイルを切り替えます。
- LogWriter.Dispose を呼び出すと強制的にストリームを開き直す事ができます。
出力される日時の書式を yyyy/MM/dd HH:mm:ss.fffffff に変更しました。
CreateLogRow を LINQ から for に変換しました。

その他 / Other information

ghost · 2021-03-20T00:13:53Z

実際に試していないですが、#38 (comment) にある
Rotate の処理やログ送信の処理の時には、削除するファイル(こちらはあまり状況としてありえないかも)や
ログ送信するファイル(ZipFile.CreateFromDirectory の対象ファイル)がオープンされたままで問題ないのかの
確認は済んでいますか？

あと、コードフォーマットや命名規則が他のファイルと全く違っているので、できればある程度
揃えておいた方がよいのではないかなと思いました

見落としていましたが書式は勝手に変えたらダメですね。運用側ではこのログを見ている人がいるはずなので。。

Takym · 2021-03-20T09:47:09Z

Rotateやログ送信時はIDEの不具合（#53）でまだ試せていません。ファイルの共有はされるので理論上は動くと思っています。

コードフォーマットについては、マージ直前に修正する予定です。その時には .editorconfig ファイルを貸して頂ければ助かります。

Takym · 2021-05-03T08:02:34Z

排他制御を Interlocked を使う形に修正しました。確認の程、宜しくお願い致します。

keiji · 2021-05-11T03:00:59Z

この変更について、ぼくはCOCOAの改善として良いと考えているのでレビューしていきます。

大きい変更なので、ぼくがApproveしてから開発チーム側でレビューする流れになります。ぼくのApprove != 取り込み決定ではないことをお伝えしておきます。

処理効率を上げた結果、デグレが起こってログが失われるのが一番よろしくないので、そのあたりの懸念をなくすためにどうするのが良いかが課題ですね。

keiji

所見でレビューコメント付けました。
ひとまず、フォーマッターの変更をお願いします！

Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

keiji · 2021-05-11T03:13:25Z

CONTRIBUTORS.md

@@ -75,6 +75,7 @@
 - Tassana Thaveeteeratham (Thai Translation)
 - Kotaro Sakamoto
 - Koichi Yokota (Documentation)
+- Takym (LogWriter)


CONTRIBUTORに関しては新しく項目を設けますね。

keiji · 2021-05-11T03:14:55Z

Covid19Radar/Covid19Radar/Services/Logs/LoggerService.cs

@@ -1,18 +1,13 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/* This Source Code Form is subject to the terms of the Mozilla Public


この変更でBOMが取れたりしていますか？
もし取れていれば付け直してもらえると（付けてあれば全く問題ないです

Visual Studio で保存した場合、BOM が付いたり消えたりしました(そろそろ BOM 無しを既定の設定にして欲しいですよね)。VSCode から BOM を取り除きました。

Covid19Radar/Covid19Radar/Services/Logs/LoggerService.cs

Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

keiji · 2021-05-11T03:26:45Z

Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

+ );
+ }
+
+ private static string CreateLogRow(params string[] cols)


元実装と比べると複雑度が増しているように思います。
特定文字があったら置き換える処理と理解していますが、元実装のReplaceを使った方法で不都合は想定されますか？

置き換え文字列が多い場合は変換テーブルのようなものを作ってそれに基づいて処理をする（処理そのものはすっきりさせる）のがうれしいです。ご意見をうかがえればと思います。

LINQ から for に替えて速度向上を図ろうとしました。後で性能比較します。（Replace と同時に Select も展開しています。）

~~簡略化して試してみた所、LINQ より FOR の方が微妙に早くまたコードサイズも小さくなりました。~~

https://gist.github.com/Takym/e0a382ff63179c5a7b07bfc301e2914b

元の実装と本PRの実装の性能を比較しました。全体的に向上しています。

ここは可読性優先でいきましょう！ミリ秒単位の差なら気になるところですが、ナノ秒なら無視しても良いレベルだと思います。

こういう提案があったことは記録として残すので、性能が問題になったら採用と言うことで。

cocoa/Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

Lines 145 to 182 in 36b68b0

private static string CreateLogRow(params string[] cols)

{

var sb = new StringBuilder();

for (int i = 0; i < cols.Length; ++i) {

if (i != 0) {

sb.Append(", ");

}

string s = cols[i];

sb.Append('\"');

for (int j = 0; j < s.Length; ++j) {

char ch = s[j];

switch (ch) {

case '\t':

sb.Append("\\t");

break;

case '\v':

sb.Append("\\v");

break;

case '\r':

sb.Append("\\r");

break;

case '\n':

sb.Append("\\n");

break;

case '\\':

case '\"':

sb.Append(ch);

sb.Append(ch);

break;

default:

sb.Append(ch);

break;

}

}

sb.Append('\"');

}

return sb.ToString();

}

単位はナノ秒で書かれていますが、差は約0.5マイクロ秒程あります。
ログ出力処理は様々な場所で沢山呼ばれており、この小さい差は全体的には大きな影響を与えると思います。

今は可読性と処理速度を両立したコードを試行錯誤しています。少し時間が掛かりそうです。すみません。

締め切りとかがあるわけではないので大丈夫ですよ。

ぼくの考えとして、このログ出力は必ずFile IOを伴うものです。ファイル入出力にかかる時間を考えると、提示いただいているナノ秒・マイクロ秒未満の差は誤差の域だとぼくは思っていて、現時点であまり頑張って縮める必要はないとも考えています。

ぼく自身、Androidではカスタムビューで60fpsを維持したまま複雑な描画処理をするのにどうするかとか、カメラプレビューの大きな画像データを可能な限り早く前処理して機械学習モデルに入力するとかしているので、場合によってはナノ秒単位の差が積み重なってアプリの体験として命取りになることは承知しています。

その上で、この部分はそこまでのことではないなと考えています。最初の方にいただいていた「ファイルハンドラーを開いて保持しておく。日付が変わっていたら新しいファイルで開き直す」があれば、Issueそのものは解決できると考えています。

処理速度については、現在、ファイルハンドラーを開いたり閉じたりするやり方でも、性能面で問題になっていないると言う認識はありません。また、ハンドラーを開いたり閉じたりしなくなる分、少なくとも性能は上がるものと期待しているので、性能については今後、問題になるようなことがあれば、そのときに対応すればいいと考えています。

工夫して可読性と効率のバランスを取る事ができました。

for 文を foreach 文に置き換えました。

配列に対する foreach は for と同程度の性能を持つそうです。

switch を文から式に変更して、何を何に変換しているか分かり易くしました。

この書き方は switch 式や Replace よりも見やすいと思います。

'変換前の文字' => '変換後の文字列'

三項演算子と破棄を利用して、左側に条件、右側に値を配置しました。

三項演算子は式ですので破棄は必ず書かなくてはなりません。

カンマを入れるかどうかの判定を無くし行数を減らしました。

動作効率も僅かながら向上しています。

動作効率については以下の点を気を付けました。

StringBuilder を使いました。

元のコードと比べると、文字列を追加する順番も明確になり、可読性も高いと感じています。

ループの深さを最小限にしました。

元のコードでは Select、Replace、Join を使っており、ループ回数が多いです。

文字列連結演算子は使わない様にしました。

文字列連結演算子を使うと、メモリの確保が発生し、全体的に効率が落ちてしまいます。

cocoa/Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

Lines 139 to 160 in 72a4f73

private static string CreateLogRow(params string[] cols)

{

_ = _sb is null ? _sb = new StringBuilder()

: _sb.Clear();

foreach (string col in cols) {

_sb.Append(",\"");

foreach (char ch in col) {

string? escaped = ch switch {

'\t' => "\\t", '\v' => "\\v",

'\r' => "\\r", '\n' => "\\n",

'\\' => "\\\\", '\"' => "\"\"",

_ => null

};

_ = escaped is null ? _sb.Append(ch)

: _sb.Append(escaped);

}

_sb.Append('\"');

}

_sb.Remove(0, 1);

return _sb.ToString();

}

LINQ を使わずに書いてみましたが、どうでしょうか？

Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

Co-authored-by: ARIYAMA Keiji <keiji.ariyama@gmail.com>

Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

Takym · 2021-05-11T08:22:29Z

Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

+ );
+ }
+
+ private static string CreateLogRow(params string[] cols)


LINQ から for に替えて速度向上を図ろうとしました。後で性能比較します。（Replace と同時に Select も展開しています。）

Takym · 2021-05-11T09:30:10Z

Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

+ private void WriteLine(DateTime jstNow, string line)
+ {
+ var file = this.file;
+ string fname = logPath.LogFilePath(jstNow);
+ if (file is null || file.FileName != fname) {
+ var newFile = new File(fname, enc);
+ RewriteField(ref this.file, newFile, ref file);
+ file?.Dispose();
+ file = newFile;
+ file.Writer.WriteLine(HEADER);
+ }
+ file.Writer.WriteLine(line);
+ }
+
+ /// <inheritdoc/>
+ public void Dispose()
+ {
+ var file = this.file;
+ RewriteField(ref this.file, null, ref file);
+ file?.Dispose();
+ }
+
+ [MethodImpl(MethodImplOptions.AggressiveInlining)]
+ private static void RewriteField<T>(ref T field, T newValue, ref T oldValue) where T: class?
+ {
+ while (Interlocked.CompareExchange(ref field, newValue, oldValue) != oldValue) {
+ Thread.Yield();
+ oldValue = field;
+ }
+ }


while (true) を無くし、処理を共通化しました。
file への書き換えが発生するタイミングは下記の四つです。

始めて WriteLine(DateTime, string) が呼ばれた時

日付が変わった時

Dispose が呼ばれた時

Dispose 後に WriteLine(DateTime, string) が呼ばれた時

複数のスレッドが同時に80行目に到達しますと、同じログファイルに HEADER が複数出力される可能性はあります。また、HEADER が出力される前に line が出力される可能性もあります。

複数のスレッドが同時に80行目に到達しますと、同じログファイルに HEADER が複数出力される可能性はあります。また、HEADER が出力される前に line が出力される可能性もあります。

ここだけ見るとけっこう厳しい挙動だなと思います。
WriteLine自体を同期化というか、SemaphoreSlimとかを使って２つのスレッドの処理が同時に通らないようにすれば解決しないでしょうか。

ログ出力は、そこまで性能を追い求めるものではないので、もう少しシンプルな作りの方が安心かと思います。現状、ファイルを開いたり閉じたりして効率が悪いことは確かですが、ちゃんと動いているという実績は大きいです。開発チームに推していくに当たって、現状のログ機構と同等の安定性があるとうれしいです。

~~フィールドの書き換えに成功した時にのみ HEADER を出力する様にしました。~~
~~そして、書き換えに失敗した場合は、現在開いている _file の名前が最新かどうか確認する様に変更しました。~~

cocoa/Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

Lines 81 to 90 in b4f15d8

do {

file = _file;

if (Interlocked.CompareExchange(ref _file, newFile, file) == file) {

file?.Dispose();

file = newFile;

file.Writer.WriteLine(HEADER);

break;

}

Thread.Yield();

} while (file is null || file.FileName != fname);

~~どうでしょうか？~~

WriteLine(DateTime, string) の処理を改善し、HEADER の複数回出力が発生する確率を下げました。

cocoa/Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

Lines 75 to 93 in 68b7d9f

private void WriteLine(DateTime jstNow, string line)

{

var file = _file;

string fname = _log_path.LogFilePath(jstNow);

if (file is null || file.FileName != fname) {

var newFile = new File(fname, _enc);

do {

if (Interlocked.CompareExchange(ref _file, newFile, file) == file) {

newFile.Writer.WriteLine(HEADER);

file?.Dispose();

file = newFile;

break;

}

Thread.Yield();

file = _file;

} while (file is null || file.FileName != fname);

}

file.Writer.WriteLine(line);

}

処理を詳しく説明します。

77行目でローカル変数 file にフィールド変数 _file の値を退避させます。

79行目で file が最新かどうか判定します。

最新ではない場合は下記の処理を行います。

80行目で最新のファイルオブジェクトを生成し newFile に格納します。

82行目で _file と file が等しかった時にのみ newFile に置き換えます。

等しかった場合は、HEADER を出力し、元の file を破棄します。

ループから抜けます。

等しくなかった場合は、下記の処理を行います。

89行目でもう一度、ローカル変数 file にフィールド変数 _file の値を退避させます。

90行目で file が最新かどうか判定し、最新でなければ再試行します。

最新であればループから抜けます。

92行目でログをファイルに書き込みます。

どうでしょうか？

追記

HEADER を _file フィールドの書き換え直後に行う様にしました。

_file フィールドの書き換えに失敗した時に、現在の _file が最新かどうか確認していますので、HEADER が複数回出力される事は無い筈です。

Takym · 2021-05-22T10:25:33Z

CocoaLogViewer で目視で確認した所、正しくログ出力できていました。

Takym · 2021-06-05T12:56:12Z

@keiji 返信が遅れて申し訳ありません。

非公開変数の命名規則について

現在、非公開になっているフィールド変数の命名規則は統一されていない様に見えます。
現段階で変数名を変更したとしても、命名規則を統一する時にもう一度変更し直す可能性もあります。
本PRの目的は命名規則の統一ではありませんので、今回はこのままにさせてください。
（一部の変数名は修正しました。）

内部クラス `File` について

このクラスは LogWriter の外側からは参照できない様に設定してあるため短い名前になりました。
ただ、System.IO.File と同名であり紛らわしいので LogFile と改名します。
また、内部構造(下記の点)を変更しました。

StreamWriter を外部から直接触れない様にしました。
StreamWriter が生成されているか確認してから破棄する様にしました。

必要であれば、このクラスは LogWriter の外へ移動して外部から注入できる様にしても良いかもしれません。

`CreateLogRow` について

こちらについては新しく Issue を作成しました(#208)。
ただ、余分なコミットを作成しない様、このPRは分割せず目的を追加する事にします。

テスト失敗について

Takym#4 (comment) に移動しました。

ログを見る限り下記の処理で失敗している様です。

cocoa/Covid19Radar/Tests/Covid19Radar.UnitTests/Services/Logs/LoggerServiceTests.cs

Line 44 in bc848ce

using (var sr = new StreamReader("~/.cocoa/logs/cocoa_log_20201101.csv"))

cocoa/Covid19Radar/Tests/Covid19Radar.UnitTests/Services/Logs/LoggerServiceTests.cs

Line 88 in bc848ce

Mock.Get(mockILogPathService).Verify(s => s.LogsDirPath, Times.Exactly(2));

cocoa/Covid19Radar/Tests/Covid19Radar.UnitTests/Services/Logs/LoggerServiceTests.cs

Line 107 in bc848ce

Mock.Get(mockILogPathService).Verify(s => s.LogsDirPath, Times.Once());

cocoa/Covid19Radar/Tests/Covid19Radar.UnitTests/Services/Logs/LoggerServiceTests.cs

Line 126 in bc848ce

Mock.Get(mockILogPathService).Verify(s => s.LogsDirPath, Times.Once());

~~テスト処理では LogWriter を毎回生成していますので、それが原因でIO処理が上手く行っていないと予想しています。~~　→違いました。

Takym · 2021-06-05T13:11:39Z

Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

+ var jstNow = Utils.JstNow();
+ string row = CreateLogContentRow(message ?? string.Empty, method, filePath, lineNumber, logLevel, jstNow, _essentials);
+ Debug.WriteLine(row);
+ this.WriteLine(jstNow, row);


この部分では日付を取得してから、WriteLine を呼び出しています。
しかし、WriteLine を実行するタイミングで日付が変更され、且つ _log_file が最新のファイルに更新された場合に、ファイルの開き直しが複数回行われる気がします。
日付の取得は WriteLine 内に移動した方が良さそうです。

下記の可能性が考えられます。

日付が変更される前に、スレッドAが WriteLine を呼び出します。

日付が変更された後に、スレッドBが WriteLine を呼び出します。

スレッドB が _log_file を最新のファイルに更新します。

スレッドA が _log_file を前日のファイルに更新します。

別のスレッドCがログを書き込むタイミングで、もう一度 _log_file を最新のファイルに更新します。

WriteLine の91行目の直後に日付を再取得すればこの問題は発生しませんが、この場合、前日のログを保持したログファイルが生成される可能性が生まれます。

Takym · 2021-06-11T07:26:33Z

単体テストについてはこちらに情報をまとめていきます。

Takym · 2021-06-23T07:28:02Z

@keiji さん
この PR の向き先を #226 と共に feature/log_improvement へ変えて頂けませんでしょうか？宜しくお願い致します。

keiji · 2021-06-23T08:28:35Z

向き先変更しました。
CIを動かすために一度Closeとreopenします。

Takym · 2021-06-23T08:36:08Z

ありがとうございます。

keiji · 2021-07-01T02:26:04Z

feature/log_improvementを最新のdevelopにrebaseした方がいいかもしれませんね。
後でやります。

keiji · 2021-07-01T16:58:24Z

CIを動かすためにCloseとreopenします。

keiji · 2021-07-01T17:04:21Z

@Takym 今回、developの直近のcommitを一気にloggingに取り込んでいるように見えました。Pull Request一つ一つを構成する変更は、そのタイトルが指し示す内容を実現するのに必要と思われるcommitに限定してもらえるとありがたいです。

もしdevelopの更新を取り入れる必要がある場合、今回のようにrebaseをかけるので声をかけてください。

Takym · 2021-07-01T17:12:47Z

テスト処理を自動的に実行させる為に最新の CI を GitHub 上から取り込みました所、feature/log_improvement ではなくフォーク元のメインブランチである develop と同期されてしまいました。今度からは GitHub 上からではなく Git クライアントから同期する様にします。すみません。

* DeleteLogsDirIfExists 無効化 * テスト時のストリームの開き方を変更 * `DeleteLogsDirIfExists` を再度有効化＆ログファイルの削除を許可 * `CocoaLogViewer` からコードを拝借 * テストケース修正 * Revert "テストケース修正" This reverts commit 7b43750. * テストケース修正 * `ILogWriter.Write` の仕様を変更し、null 値の入力を許可。 * Update CI.yml for logging_test * Revert "Update CI.yml for logging_test" * Suppress diff * Update CI.yml for logging test * Update CIserver.yml for logging test * `LogWriter` を大幅に改善 * テスト処理修正：`StartMethod_Success_LogsDir_Not_Exists`、`StartMethod_Success_LogsDir_Exists_LogFile_Not_Exists`、`StartMethod_Success_LogsDir_Exists_LogFile_Exists` * ログファイルのヘッダーの出力タイミングを変更 * Update CI.yml * Update CIserver.yml * Update CI.yml

keiji · 2021-09-08T12:06:15Z

このPRでは #38 と #208 の二つの解決が図られています。

#38 については #67 (comment) でもお伝えしましたが、速度面の心配は現時点であまりしていないので、可読性を優先してLINQを使った方法にしてもらえればと思います。

また、#208 については、タブ文字や改行コードをエスケープついて少なくとも「動作ログを確認する」という現在の用途では必須ではなく、開発チームとの調整ができていないので、こちらも一旦外してもらえると話が進めやすくなりますが、いかがでしょうか。

Takym · 2021-09-08T15:44:29Z

（久し振りにソースコードを確認しましたので、誤っている可能性があります。）

CreateLogRow の可読性ですが #208 の対応と関係しています。恐らく #208 に対応しない場合は「可読性優先コード」でも速度面も許容範囲内だったと思いますが、「可読性優先コード（#208 未対応）」は「速度優先コード（#208 対応済み）」より約2.3倍程度遅いです（性能比較）。#208 に対応した場合の「速度＋可読性優先コード」と「可読性優先コード」を性能比較した結果、速度差は大きいという事が分かりました。うろ覚えですが1桁程の差がありましたので、約2.3倍の差が約10倍に膨れ上がりました。LINQを使った場合に速度が落ちる原因は、文字列生成時のメモリ確保だったと思います。性能比較に使った検証用のコードは時間がある時に載せておきます（最近は忙しくいつ載せれるかは分かりません）。

可読性優先コード（LINQ、元のコード、ログ出力にタブ文字や改行コードをエスケープする #208 未対応）

cocoa/Covid19Radar/Covid19Radar/Services/Logs/LoggerService.cs

Lines 220 to 229 in 3e7fdeb

private string CreateLogRow(List<string> columns)

{

var convertedColumns = columns

.Select(column => column ?? string.Empty)

.Select(column => column.Replace("\r", "").Replace("\n", ""))

.Select(column => column.Replace("\"", "\"\""))

.Select(column => "\"" + column + "\"");

return string.Join(",", convertedColumns);

}

速度優先コード（FOR、ログ出力にタブ文字や改行コードをエスケープする #208 対応済み）

cocoa/Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

Lines 137 to 173 in f4986d2

private static string CreateLogRow(params string[] cols)

{

var sb = new StringBuilder();

for (int i = 0; i < cols.Length; ++i) {

if (i != 0) {

sb.Append(',');

}

string s = cols[i];

sb.Append('\"');

for (int j = 0; j < s.Length; ++j) {

char ch = s[j];

switch (ch) {

case '\t':

sb.Append("\\t");

break;

case '\v':

sb.Append("\\v");

break;

case '\r':

sb.Append("\\r");

break;

case '\n':

sb.Append("\\n");

break;

case '\\':

case '\"':

sb.Append(ch).Append(ch);

break;

default:

sb.Append(ch);

break;

}

}

sb.Append('\"');

}

return sb.ToString();

}

速度＋可読性優先コード（FOREACH、ログ出力にタブ文字や改行コードをエスケープする #208 対応済み）

cocoa/Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

Lines 160 to 180 in ac1f49f

private static string CreateLogRow(params string[] cols)

{

var sb = _sb_cache is null ? _sb_cache = new StringBuilder()

: _sb_cache.Clear();

foreach (string col in cols) {

sb.Append(",\"");

foreach (char ch in col) {

string? escaped = ch switch {

'\t' => "\\t", '\v' => "\\v",

'\r' => "\\r", '\n' => "\\n",

'\\' => "\\\\", '\"' => "\"\"",

_ => null

};

_ = escaped is null ? sb.Append(ch)

: sb.Append(escaped);

}

sb.Append('\"');

}

sb.Remove(0, 1);

return sb.ToString();

}

「速度優先コード」との差：「ログ出力時に毎回ファイルを開いている」の修正 #67 (comment)

今回提案するコード（新規、未コミット）

private static string CreateLogRow(params string[] cols)
{
    var convertedColumns = cols
        .Select(column => column ?? string.Empty)
        .Select(column => column.Replace("\r", "").Replace("\n", ""))
        .Select(column => column.Replace("\"", "\"\""))
        .Select(column => "\"" + column + "\"");

    return string.Join(",", convertedColumns);

    //* // https://github.com/cocoa-mhlw/cocoa/issues/208 対応コード (*1)
    var sb = _sb_cache is null ? _sb_cache = new StringBuilder()
                               : _sb_cache.Clear();
    foreach (string col in cols) {
        sb.Append(",\"");
        foreach (char ch in col) {
            string? escaped = ch switch {
                '\t' => "\\t",  '\v' => "\\v",
                '\r' => "\\r",  '\n' => "\\n",
                '\\' => "\\\\", '\"' => "\"\"",
                _    => null
            };
            _ = escaped is null ? sb.Append(ch) 
                                : sb.Append(escaped);
        }
        sb.Append('\"');
    }
    sb.Remove(0, 1);
    return sb.ToString();
    //*/
}

(*1): シンタックスハイライトの為にコメントアウトを解除しています。

Takym added 3 commits March 20, 2021 07:04

LogWriter 作成

de947fc

LoggerService のコンストラクタ変更に関する修正

94b7d96

CONTRIBUTORS.md 更新

d0b4d44

Takym added 2 commits May 3, 2021 14:32

CreateLogRow を修正

36b68b0

lock を Interlocked に変更

082a089

Takym added 2 commits May 3, 2021 19:08

Merge branch 'master' into logging

016beb2

Merge branch 'logging' of https://github.com/Takym/cocoa into logging

a9b4b62

keiji self-requested a review May 11, 2021 02:40

keiji assigned Takym May 11, 2021

keiji suggested changes May 11, 2021

View reviewed changes

Update Covid19Radar/Covid19Radar/Services/Logs/LogWriter.cs

e72f0da

Co-authored-by: ARIYAMA Keiji <keiji.ariyama@gmail.com>

Takym commented May 11, 2021

View reviewed changes

Takym added 2 commits May 11, 2021 18:15

ソースファイルの書式を修正

b459354

Merge branch 'logging' of https://github.com/Takym/cocoa into logging

cfc16a0

Takym commented May 11, 2021

View reviewed changes

Takym added 2 commits May 11, 2021 18:35

CreateLogRow を微修正

a00563a

フィールド名修正

03f7fab

keiji mentioned this pull request May 17, 2021

ログ出力時に毎回ファイルを開いている #38

Open

Takym added 7 commits May 18, 2021 21:17

_file フィールドの書き換え処理を改善

a68cac6

WriteLine(DateTime, string) を改善

b4f15d8

WriteLine(DateTime, string) を微修正

2f6002d

HEADER の出力タイミングを修正

68b7d9f

テスト処理修正

f4986d2

CreateLogRow、可読性＆性能向上

186a323

可読性＆性能向上２

72a4f73

Takym added 4 commits June 5, 2021 20:55

LogWriter、フィールド変数の名前を変更

09eaa47

LogWriter.LogFile の仕組みを変更

c00b730

LogWriter.WriteLine のローカル変数の名前を変更

a4b32c0

LogWriter.LogFile.FileName を **Path** に改名

bc848ce

Takym mentioned this pull request Jun 5, 2021

ログ出力にタブ文字や改行コードをエスケープする #208

Open

Takym commented Jun 5, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

keiji changed the base branch from master to develop June 7, 2021 07:16

keiji mentioned this pull request Jun 10, 2021

featureブランチを運用する #219

Closed

Merge branch 'cocoa-mhlw:develop' into logging

af59231

Takym mentioned this pull request Jun 11, 2021

Update unit tests for the logging system. Takym/cocoa#4

Merged

keiji changed the base branch from develop to feature/log_improvement June 23, 2021 08:27

keiji closed this Jun 23, 2021

keiji reopened this Jun 23, 2021

Merge branch 'cocoa-mhlw:develop' into logging

b0a617c

keiji force-pushed the feature/log_improvement branch from af44703 to 9ada1f7 Compare July 1, 2021 16:56

keiji closed this Jul 1, 2021

keiji reopened this Jul 1, 2021

		@@ -1,18 +1,13 @@
		/* This Source Code Form is subject to the terms of the Mozilla Public
		/* This Source Code Form is subject to the terms of the Mozilla Public

	private static string CreateLogRow(params string[] cols)
	{
	var sb = new StringBuilder();
	for (int i = 0; i < cols.Length; ++i) {
	if (i != 0) {
	sb.Append(", ");
	}
	string s = cols[i];
	sb.Append('\"');
	for (int j = 0; j < s.Length; ++j) {
	char ch = s[j];
	switch (ch) {
	case '\t':
	sb.Append("\\t");
	break;
	case '\v':
	sb.Append("\\v");
	break;
	case '\r':
	sb.Append("\\r");
	break;
	case '\n':
	sb.Append("\\n");
	break;
	case '\\':
	case '\"':
	sb.Append(ch);
	sb.Append(ch);
	break;
	default:
	sb.Append(ch);
	break;
	}
	}
	sb.Append('\"');
	}
	return sb.ToString();
	}

	private static string CreateLogRow(params string[] cols)
	{
	_ = _sb is null ? _sb = new StringBuilder()
	: _sb.Clear();

	foreach (string col in cols) {
	_sb.Append(",\"");
	foreach (char ch in col) {
	string? escaped = ch switch {
	'\t' => "\\t", '\v' => "\\v",
	'\r' => "\\r", '\n' => "\\n",
	'\\' => "\\\\", '\"' => "\"\"",
	_ => null
	};
	_ = escaped is null ? _sb.Append(ch)
	: _sb.Append(escaped);
	}
	_sb.Append('\"');
	}
	_sb.Remove(0, 1);
	return _sb.ToString();
	}

	do {
	file = _file;
	if (Interlocked.CompareExchange(ref _file, newFile, file) == file) {
	file?.Dispose();
	file = newFile;
	file.Writer.WriteLine(HEADER);
	break;
	}
	Thread.Yield();
	} while (file is null \|\| file.FileName != fname);

	private void WriteLine(DateTime jstNow, string line)
	{
	var file = _file;
	string fname = _log_path.LogFilePath(jstNow);
	if (file is null \|\| file.FileName != fname) {
	var newFile = new File(fname, _enc);
	do {
	if (Interlocked.CompareExchange(ref _file, newFile, file) == file) {
	newFile.Writer.WriteLine(HEADER);
	file?.Dispose();
	file = newFile;
	break;
	}
	Thread.Yield();
	file = _file;
	} while (file is null \|\| file.FileName != fname);
	}
	file.Writer.WriteLine(line);
	}

「ログ出力時に毎回ファイルを開いている」の修正 #67

Are you sure you want to change the base?

「ログ出力時に毎回ファイルを開いている」の修正 #67

Conversation

Takym commented Mar 19, 2021 • edited Loading

Issue 番号 / Issue ID

目的 / Purpose

破壊的変更をもたらしますか / Does this introduce a breaking change?

Pull Request の種類 / Pull Request type

検証方法 / How to test

コードの入手 / Get the code

コードの検証 / Test the code

確認事項 / What to check

その他 / Other information

ghost commented Mar 20, 2021 • edited by ghost Loading

Takym commented Mar 20, 2021 • edited Loading

Takym commented May 3, 2021 • edited Loading

keiji commented May 11, 2021

keiji left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Takym May 11, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Takym May 18, 2021 • edited Loading

Choose a reason for hiding this comment

Takym May 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Takym May 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Takym May 11, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Takym May 18, 2021 • edited Loading

Choose a reason for hiding this comment

Takym May 18, 2021 • edited Loading

Choose a reason for hiding this comment

追記

Choose a reason for hiding this comment

Takym commented May 22, 2021

Takym commented Jun 5, 2021 • edited Loading

非公開変数の命名規則について

内部クラス File について

CreateLogRow について

テスト失敗について

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Takym commented Jun 11, 2021

Takym commented Jun 23, 2021

keiji commented Jun 23, 2021

Takym commented Jun 23, 2021

keiji commented Jul 1, 2021

keiji commented Jul 1, 2021

keiji commented Jul 1, 2021

Takym commented Jul 1, 2021 • edited Loading

keiji commented Sep 8, 2021

Takym commented Sep 8, 2021

Takym commented Mar 19, 2021 •

edited

Loading

ghost commented Mar 20, 2021 •

edited by ghost

Loading

Takym commented Mar 20, 2021 •

edited

Loading

Takym commented May 3, 2021 •

edited

Loading

Takym May 11, 2021 •

edited

Loading

Takym May 18, 2021 •

edited

Loading

Takym May 18, 2021 •

edited

Loading

Takym May 22, 2021 •

edited

Loading

Takym May 11, 2021 •

edited

Loading

Takym May 18, 2021 •

edited

Loading

Takym May 18, 2021 •

edited

Loading

Takym commented Jun 5, 2021 •

edited

Loading

内部クラス `File` について

`CreateLogRow` について

Takym commented Jul 1, 2021 •

edited

Loading