CGrepAgent::SetGrepResult 出力形式がノーマルの場合の処理の高速化 #418
Conversation
sakura_core/CGrepAgent.cpp
Outdated
| p += s(nColumn, p); | ||
| *p++ = L')'; | ||
| *p = NULL; | ||
| cmemBuf.AppendString(strWork); |
There was a problem hiding this comment.
ごめんなさい、既存の枠を使って速くする手段がありそうに思ってます。
既存の枠を使って簡単に書き換え：
cmemBuf.AppendStringF( L"(%I64d,%d)", nLine, nColumn );auto_sprintf の余分なオーバーヘッドと文字列コピーのオーバーヘッドを排除できるのでこれだけでも数%の改善を見込める気がします。
There was a problem hiding this comment.
Performance Profiler を使って計測してみました。
Win32 Releaseビルドで、リポジトリフォルダを以下の設定で Grep しました。
- 条件：a
- ファイル：*.cpp *.c *.h *.hpp *.hh *.cc
- サブフォルダからも検索するにチェック
各3回、起動してからGrepして終了、という手順でProfile取っています。
Profileに起動処理も含めているので、Grep処理だけでいうとパーセンテージはもっと
大きくなります。
CGrepAgent::SetGrepResult の該当する処理の箇所のProfile結果を貼り付けます。
現時点の master (82724c0)
このPR (21326ab)
#418 (comment) 提案手法
There was a problem hiding this comment.
この検証結果から分かることは、
ぼくの指摘内容に若干の効果があることと
beruさんのPRによる速度改善効果がケタ違いであることだと思います。
| リポジトリ | 平均値 |
|---|---|
| master | 63(3.71%) |
| ぼくの指摘 | 50(2.98%) |
| beruさんのPR | 12くらい(1%未満) |
比較してる処理内容が同じじゃないんで厳密な比較はできないんですがｗ
いちおう、改善効果があることは分かったうえのダメ出しであることをお伝えしときます。
このPRをリポジトリに入れると、似たようなテンプレ関数をあちこちにコピペでいれにゃならんハメになると思うからです。
コーディングの際はできるだけ Don't Repeat Yourself 原則 に気を遣い、Single Responsibility Principal を果たせるような構造を目指すのが理想だと思ってます。
There was a problem hiding this comment.
このPRをリポジトリに入れると、似たようなテンプレ関数をあちこちにコピペでいれにゃならんハメになると思うからです。
それは自分も危惧していました。なので数値を文字列化する関数をどっか別のヘッダファイルに置いた方が良いんじゃないかとか少し思いましたし、そういう指摘が来るかもしれないと思ってました。
汎用化と特殊化による効率化って両立が難しいですね（poem
C++erだとstd::algorithmとファンクタを組み合わせて複雑な処理をコンパクトに書けるかもしれないですが、staticおじさんなのでベタにforループで書いておくれ…と思ったりします（再poem
コーディングの際はできるだけ Don't Repeat Yourself 原則 に気を遣い、Single Responsibility Principal を果たせるような構造を目指すのが理想だと思ってます。
日本の夏は湿度が高いのでその原則は適用しづらいです。
sakura_core/CGrepAgent.cpp
Outdated
| @@ -878,8 +878,36 @@ cancel_return:; | |||
| return -1; | |||
| } | |||
|
|
|||
| // org : https://stackoverflow.com/a/12386915/4699324 | |||
| template <typename T, typename ChT> | |||
| int s(T value, ChT *sp) | |||
sakura_core/CGrepAgent.cpp
Outdated
| if (i < 10) | ||
| *tp++ = (ChT)(i + '0'); | ||
| else | ||
| *tp++ = (ChT)(i + 'a' - 10); |
There was a problem hiding this comment.
確かに不要ですね。レビューありがとうございます。
sakura_core/CGrepAgent.cpp
Outdated
| } | ||
|
|
||
| int len = tp - tmp; | ||
| if (value < 0) { |
There was a problem hiding this comment.
確かにそうなのですが（行番号と桁番号で負数が無い事はコメントでも書かれている）
- 変数の型は符号有り
- 文字列化のループに比べるとそこまでコストが高くない
という事でそのままでも良いのではないかと思います。
There was a problem hiding this comment.
while (tp > tmp) の部分のループは負の値をサポートするために必要になっています。
この関数は汎用性を捨てることで、速度を稼ぐために存在しています。
速度を稼ぐのが目的なら、必要のない汎用性は捨てるべきだと思います。
またテンプレートにする必要もないと思います。
There was a problem hiding this comment.
while (tp > tmp) の部分のループって（下位の桁から文字列化して一時変数に出力しているので）本当の出力先に文字列を反転して書き出している処理という認識です。
負の値のサポートは確かに必要ない筈なので関数の入り口で assert で負じゃない事を確認して、不要な処理は省いた方が良いかもしれないですね。
数値の型が 行番号が LONGLONG で 桁番号が int で別々なので template 関数にしています。文字の型は wchar_t 固定なのでそこは template 引数にする必要は無いとは思います。
There was a problem hiding this comment.
あと、符号ありの仕様のままにするにしても、整数部分の変換をする前に先に符号の判定を
行って最初に '-' を出力するようにすれば、while (tp > tmp) のループは削除できます。
There was a problem hiding this comment.
あと、符号ありの仕様のままにするにしても、整数部分の変換をする前に先に符号の判定を
行って最初に '-' を出力するようにすれば、while (tp > tmp) のループは削除できます。
負の場合にマイナス文字を先に書き出す事自体は良いと思いますが、 while (tp > tmp) のループは文字列の反転に必要です。
There was a problem hiding this comment.
ややこしくなりますが、
例えば 64bi の変数を 10進数で出力したときの最大桁数以上のバッファを用意した上で
後ろから先頭にデータを詰めていって、先頭アドレスのポインタを返せば無駄に
コピーしなくてよくなります。
あと、value の値が 0 のときに正しく動作しません。
while 文ではなく do - while 文にしたら動くようになると思います。
@berryzplus さんも書いているように同様の最適化をすると思うので、高速化のために
同様の文字列関数が作られていくことになるかもしれないので、ユーティリティを
入れるためのファイルを用意して、再利用できるようにした方がいいです。
実装を見て、関数仕様を想像して、というスタンスのようですが、
明確にどんな関数仕様なのかをコメントで明示した方がいいと思います。
WIP の PR なので後でやるというのなら、省略するのもありですが、
WIP ではない PR ならそこら辺は記載してほしいです。
value の値が 0 のときに正しく動作しないことを書いたように
どんな場合にどんな結果になるのかが確認、デグレ確認できるように
単体テストを書いた方がいいと思います。
There was a problem hiding this comment.
例えば 64bi の変数を 10進数で出力したときの最大桁数以上のバッファを用意した上で
後ろから先頭にデータを詰めていって、先頭アドレスのポインタを返せば無駄に
コピーしなくてよくなります。
間違いです。これは無理でした。
There was a problem hiding this comment.
@berryzplus さんも書いているように同様の最適化をすると思うので、高速化のために
同様の文字列関数が作られていくことになるかもしれないので、ユーティリティを
入れるためのファイルを用意して、再利用できるようにした方がいいです。
他のところでも使いたくなったら、後のPRで対応すれば良いかなと思っていましたがこのPRで整備した方が良いでしょうか？
実装を見て、関数仕様を想像して、というスタンスのようですが、
明確にどんな関数仕様なのかをコメントで明示した方がいいと思います。
WIP の PR なので後でやるというのなら、省略するのもありですが、
WIP ではない PR ならそこら辺は記載してほしいです。
コメントについては記載が面倒で書いていませんでした。しかし他の人がレビューする手間を考えるときちんと書いておかないと恨まれそうですね…。
There was a problem hiding this comment.
0 の場合に正しく文字列化されるように指摘通りに do while ループに変えて修正しました。
#418 (comment) にも書きましたが、
std::numeric_limits<int32_t>::min()
std::numeric_limits<int64_t>::min()等の時に正しく動作しない事が分かりました。あと数値の型が uint32_t や uint64_t の場合にコンパイルエラーが出る事も分かりました。
今回の CGrepAgent::SetGrepResult から呼び出すケースでは問題になりませんが、共通ヘッダにおいて他からも使うとなると（コーナーケースですが）対処した方が良いかもしれません。
あと単体テストを書くとなると、sakura_core/CGrepAgent.cpp に置いたままだと参照出来ないので他のファイルに移すことになり、他のファイルに移すと他からも呼べるようになる（なってしまう…）ので細かい不具合を直した方が良いかもしれません。
sakura_core/CGrepAgent.cpp
Outdated
| T v = abs(value); | ||
|
|
||
| while (v) { | ||
| T i = v % 10; |
There was a problem hiding this comment.
変数名の付け方が、いけてないと思います。
変数名を見ただけで意味がわかる、あるいはコメントで補足するようにしてほしいです。
There was a problem hiding this comment.
while ループ内でローカル変数を宣言しないようにしました。
10進数限定という事もあり式だけで処理内容が自明なのでローカル変数を使う意味がないので。
There was a problem hiding this comment.
関数仕様のコメントがほしいです。
引数、戻り値の仕様、あと printf でできるところを敢えて自前実装する理由がほしいです。
sakura_core/CGrepAgent.cpp
Outdated
| @@ -921,8 +949,14 @@ void CGrepAgent::SetGrepResult( | |||
| cmemBuf.AppendString( L"・" ); | |||
| } | |||
| cmemBuf.AppendStringT( pszFilePath ); | |||
| ::auto_sprintf( strWork, L"(%I64d,%d)", nLine, nColumn ); | |||
| cmemBuf.AppendString( strWork ); | |||
| wchar_t* p = strWork; | |||
There was a problem hiding this comment.
速いけどわかりにくいコードをインライン関数を使うなどして、速度を落とさないようにしながら、隠蔽化してほしいです。
There was a problem hiding this comment.
また速いけどわかりにくいコードを書くときは、遅いけどわかりやすいコードも
同時に書いて、両者が一致することを確認する単体テストを書く、あるいは
Debug 版に限って両方実行して、両者が一致することを確認するのが定石です。
There was a problem hiding this comment.
lineColumnToString 関数を用意してそこで処理するようにしました。
デバッグビルド時には元のコードと処理結果が同じかどうか確認するようにしました。
|
説明欄にもっとわかりやすい説明を書いてほしいです。 |
sakura_core/CGrepAgent.cpp
Outdated
| *p++ = L','; | ||
| p += int2dec(nColumn, p); | ||
| *p++ = L')'; | ||
| *p = NULL; |
There was a problem hiding this comment.
NULL は意味が違います。
NULL は NULL ポインタを表します。NULL 文字ではありません。
There was a problem hiding this comment.
null character ('\0') に記述を変更しました。
There was a problem hiding this comment.
lineColumnToString 関数の中にも同様の記述がありましたが、指摘いただいた通り対応が漏れていたので修正しました。
There was a problem hiding this comment.
NULLは #define NULL 0 で、 const int の定数値なので '\0' と同じ意味だと思います。
ただ、あえて '\0'やL'\0'` と書くことで、人間による誤読を防ぐ効果があるような気がします。
厳密には windows プログラミングで多用される NULL は多くの場合 nullptr に書換えすべき・・・
という主張もときどき見かけますね。
There was a problem hiding this comment.
値は同じでも表記を使い分けた方が良いのはその通りですね。
NULL を C++11 から入った nullptr に書き換えた方が良いというのはまぁそうなんですけど見返りが null というか void というか…。
There was a problem hiding this comment.
NULLは #define NULL 0 で、 const int の定数値なので '\0' と同じ意味だと思います。
C++ ではそうですが、
C では #define NULL ((void*)0) でした。
C++ だと C の定義だといろいろ不都合があるというのでその定義になっているだけで
意味が変わったのではないです。
|
なんかペアプログラミングみたくなってますねｗｗ 数値を文字列化するテンプレ関数の置き場所は sakura_core/util 配下のどっかとかでどうでしょう。 あと、変更対象の処理のすぐ下に WZ風向け でほぼ同じコードがあるんですが、これは対象にしなくていい感じなんですかね？ |
そうですね、細かくレビューして頂いて助かってます。
他の箇所でも使う為にこのPRで整備した方が良いですか？他のところでも使いたくなったら、後のPRで対応すれば（先延ばしすれば）良いかなと思っていました。
呼び出し元で処理内容に応じて十分な大きさのバッファを用意して使う前提で考えていました。もし考慮が漏れて不具合が生じたら誰かにデバッグを楽しんでもらえればと…。
誰か使ってる人いるんでしょうか？ |
先延ばしでもいいです。
レガシーですね。 日本語訳： 負の遺産
「自分は使わないので（検証できないので）対応しない」で理屈は通るので対応しなくて大丈夫です。 |
sakura_core/CGrepAgent.cpp
Outdated
| @@ -909,7 +967,7 @@ void CGrepAgent::SetGrepResult( | |||
| { | |||
|
|
|||
| CNativeW cmemBuf(L""); | |||
| wchar_t strWork[64]; | |||
| wchar_t strWork[128]; | |||
There was a problem hiding this comment.
このバッファサイズを変更した根拠って指摘ですか？ （指摘見返せばいいのかも知れませんけどｗ
L"(%I64d,%d)" を書式化するために必要なバッファは、
1 + 20 + 1 + 10 + 1 = 33 なので 64 で十分足りる気がしています。
There was a problem hiding this comment.
指摘ではなくて自分の変更です。もし溢れて暴走したら嫌だなぁと思って計算せずにこれぐらい増やしておけば大丈夫だろうと思って変えておきました。大は小を兼ねるという事で 1024 ぐらいにした方が良かったですね。
There was a problem hiding this comment.
e6cc577 で配列のサイズを元に戻しました。
@berryzplus さんの許可は頂けたので後は @m-tmatma さんが許可していただければ、 数値を文字列化するテンプレ関数をどっか別の場所に移すのは先延ばしに出来ればと思います。 単体テストを書くとなると参照できるようにする為に引っ越しする必要が有ります。 std::numeric_limits<int32_t>::min()
std::numeric_limits<int64_t>::min()等で正しく文字列化されない不具合があるのと、uint32_t とか uint64_t だと abs 呼び出しで
変更を局所的に隔離する事で品質を担保しましょう。きっと将来もっと良い実装に誰かが置き換えてくれるに違いありません。
それをすてるなんてとんでもない！ |
少なくともコンピュータ業界では主にマイナスな意味で使われますが、 最近は違ういい意味でも使われるみたいです。 |
まぁそこまで親切設計にしないでも良いかなと思います。引数が増えるとコメントも書かないといけませんし。 |
|
d1f7b3b で
|
sakura_core/util/string_ex2.h
Outdated
| } | ||
|
|
||
| while (tp > tmp) | ||
| *sp++ = *--tp; |
There was a problem hiding this comment.
この部分、++ と -- は別の文に分けた方がいいと思います。
行数を節約するよりも意図が明確であるようにした方がいいです。
またコードのブロック単位で何をしようとしているかのコメントが欲しいです
114 行目はまあいいかなと思いますが。
There was a problem hiding this comment.
load と store の記述行を分けるようにしました。
sakura_core/util/string_ex2.h
Outdated
| ChT tmp[64]; | ||
| ChT *tp = tmp; | ||
|
|
||
| bool isMin = (value == std::numeric_limits<T>::min()); |
There was a problem hiding this comment.
過去のコメント でも記載しましたが、
std::numeric_limits<int32_t>::min()
std::numeric_limits<int64_t>::min()を渡すと正常な結果を作れない問題に対する対策です。
abs に 符号付きの型の負数の最小値を渡すと、結果が表現できないのでC言語の仕様上は未定義のようです。なので最小値の場合は abs 前に 1 足す事で abs で処理出来るようにし、また10進数の文字列を生成後に1桁目の文字に 1 足す事で正しい値に直しています。
tests/unittests/test-int2dec.cpp
Outdated
| #include "../../sakura_core/util/string_ex2.h" | ||
|
|
||
| template <typename T> | ||
| void test_int2dec(T value, int lenAnswer, const wchar_t* strAnswer) |
There was a problem hiding this comment.
answer より expected というのが自然だと思います。
関数の期待値を指定するからです。
There was a problem hiding this comment.
resultに一票。
answerはメッセージボックスとかのユーザ入力を格納するのによく使います。
expectedはテストコードで期待値を渡すときによく使います。
resultは出力値を表すときによく使います。
There was a problem hiding this comment.
ああ、testだからexpectedがいいのかｗ
There was a problem hiding this comment.
EXPECT_EQ マクロとか使っているので expected が適してるっぽいですね。
tests/unittests/test-int2dec.cpp
Outdated
| EXPECT_STREQ(buff, strAnswer); | ||
| } | ||
|
|
||
| TEST(test, zero) |
There was a problem hiding this comment.
test という名前ですが、
int2dec あるいは それを含む値にしたらいいと思います。
名前から何のテストかわからないです。
There was a problem hiding this comment.
tests/unittests/test-sample.cpp の記述をコピペしていました。
https://github.com/google/googletest/blob/master/googletest/docs/primer.md#simple-tests
指摘いただいた通りちゃんと名前つけた方が良いですね。
| test_int2dec<int32_t>(std::numeric_limits<int32_t>::max(), 10, L"2147483647"); | ||
| test_int2dec<int64_t>(std::numeric_limits<int64_t>::max(), 19, L"9223372036854775807"); | ||
| } | ||
|
|
There was a problem hiding this comment.
値が 1, 12, 123, 1234, 12345 とか通常の正常値の値のテストもほしいです。
↑ 上記は変換後の桁数が 1 桁、2桁、... という観点です。
同様に上記でマイナスの場合もあった方がいいです。
There was a problem hiding this comment.
TEST(int2dec_test, group_sequence) で追加しました。
There was a problem hiding this comment.
負数の試験を 8b5a134 で追加しました。
There was a problem hiding this comment.
ありがとうございます。
group_sequence で同じテストがされるようになって
plus_minus_1 のテストがいらなくなったので削除お願いします。
tests/unittests/test-int2dec.cpp
Outdated
| #define NOMINMAX | ||
| #include <Windows.h> | ||
| #include <tchar.h> | ||
| #include "../../sakura_core/basis/primitive.h" |
There was a problem hiding this comment.
#422 で sakura_core のパスまでをインクルードディレクトリ指定できるようにしました。
There was a problem hiding this comment.
ありがとうございます。お陰で ../../sakura_core/ は削れるようになったので削っておきました。
sakura_core/util/string_ex2.h
Outdated
|
|
||
| // reverse digits | ||
| while (tp > tmp) { | ||
| ChT d = *--tp; |
There was a problem hiding this comment.
そういう意味ではないです。
*--tp と *sp++ です。
以下のように分けるという意味です。
tp--;
*sp = *tp;
sp++;
++ と -- は独立して使わないと意味を理解するのにすこし時間がかかりますし
コードを書いた人以外がメンテするのが難しくなります。
本当は以下も分けた方がいいと思いますが、イディオム的な書き方なのでまあいいかということです。
*sp++ = '-';
sakura_core/CGrepAgent.cpp
Outdated
| */ | ||
| static inline | ||
| wchar_t* lineColumnToString( | ||
| wchar_t* strWork, /*!< [out] 出力先 */ |
There was a problem hiding this comment.
この関数に書込み可能バッファを付ける話ってどうなりました？
template <size_t nCapacity>
static inline
wchar_t* lineColumnToString(
wchar_t( &strWork )[nCapacity],とすれば引数を増やさなくても nCapacity でデバッグアサート仕込めると思うんですが。
tests/unittests/test-int2dec.cpp
Outdated
| #include <Windows.h> | ||
| #include <tchar.h> | ||
| #include "basis/primitive.h" | ||
| class CNativeA; |
There was a problem hiding this comment.
CNativeA と CNativeW の先行宣言はここに書くべきことではないですね。
string_ex2.h の以下の箇所で、CNativeA と CNativeW が参照されているのだから
別 PR で、string_ex2.h の不具合として CNativeA と CNativeW の先行宣言を string_ex2.h に足すべきですね。
sakura/sakura_core/util/string_ex2.h
Lines 41 to 43 in 8164a93
There was a problem hiding this comment.
string_ex2.h の不具合なのかというと、そうでもないと思いますが string_ex2.h 側で対処されていると #include する側は楽ですね。
There was a problem hiding this comment.
string_ex2.h の不具合
<string.h> に LimitStringLength なんて関数ありましたっけ？
There was a problem hiding this comment.
以下では CEol に対しては先行宣言しているのに、 CNativeA と CNativeW に対しては
漏れているので対応が一貫していないので不具合という認識です。
sakura/sakura_core/util/string_ex2.h
Line 27 in 8164a93
There was a problem hiding this comment.
<string.h> に LimitStringLength なんて関数ありましたっけ？
<string.h> には無いですが、 sakura_core/util/string_ex2.cpp に実装があります。
There was a problem hiding this comment.
#423 で先行宣言足しときました。
rebase して tests/unittests/test-int2dec.cpp には先行宣言は書かないようにしました。
There was a problem hiding this comment.
util/string_ex.hにCNativeWの先行宣言を追加した理屈でいくと
basis/primitive.hのインクルードも「util/string_ex.hに追加すべき」となる気がします。
string_ex.hはprimitive.hのdefine ACHARに依存していて、
ACHARを先行定義しないとコンパイル通りません。
There was a problem hiding this comment.
全部を完璧に対応する必要はないと思っています。
対応をお願いしてる部分に対処いただければそこで一旦終わりと認識いただいて大丈夫です。
ACHARの件は対応不要です。
sakura_core/util/string_ex2.h
Outdated
| static_assert(std::is_signed_v<T>, "T must be signed type."); | ||
|
|
||
| // temporary buffer | ||
| ChT tmp[64]; |
There was a problem hiding this comment.
ほぼ同じなんですけどこれも出力バッファサイズあったほうがいいです。
template <typename T, typename ChT, size_t capacity>
int int2dec(
T value, //!< [in] the integer value to stringify
ChT ( &sp )[capacity] //!< [out] the destination string to store the stringified result
)宣言が上記のようになってれば ChT tmp[capacity]; と書けるような。
There was a problem hiding this comment.
そうすると固定長配列を使う事が前提になるので今 lineColumnToString で呼び出しているような使い方が出来なくなりませんか？
チェック機能付きのオーバーロードとか用意する手もあると思いますが、現状では使ってなくて現時点では不要です。
There was a problem hiding this comment.
そうすると固定長配列を使う事が前提になるので今 lineColumnToString で呼び出しているような使い方が出来なくなりませんか？
確認しました。対応不要です。
sakura_core/util/string_ex2.h
Outdated
| } while (v); | ||
|
|
||
| // postadjuist the lowermost digit letter | ||
| tmp[0] += isMin; |
There was a problem hiding this comment.
整数型とbool型で直接演算してええの？という疑問はありますがコンパイラが警告出してないし処理結果も正しいようなので別にいいかなと判断しました。
bool の値が C++規格上 1 or 0 になる事が決まっているとどっかで読んだ気がしたのも一因です。
https://stackoverflow.com/a/4276239/4699324
There was a problem hiding this comment.
コードの意図が理解できませんでした。年かなぁ...orz
tmp[0] += (isMin ? 1 : 0); // 符号付き最小値は -1 された値になってしまうので調整するこういうことですよね？
There was a problem hiding this comment.
自分で書いててなんですがちょっとコードが分かりにくいですよね。3日後の自分も @berryzplus さんと同じ印象を抱いていると思います。
There was a problem hiding this comment.
できれば修正をお願いします。
コメントまでコピるかどうかの判断はお任せします。
There was a problem hiding this comment.
のコメントに解説を書きましたので参照お願いします。
There was a problem hiding this comment.
了解です。加算する値が 1 or 0 であることが明確になるようにする修正はお願いしたいです。
There was a problem hiding this comment.
加算する値が 1 or 0 であることが明確になるようにする修正はお願いしたいです。
修正というのは具体的にはどういう内容をリクエストされていますか？
解説のコメントを入れる事ですか？
それとも型を bool から int に変えたり、余計な bool ? 1 : 0 を記述する事ですか？
それらは不要と判断してあえて記載しませんでした。必要だという事を示す明確な根拠は特に求めてはいませんが、リクエスト内容は明確にしていただきたいです。
There was a problem hiding this comment.
tmp[0] += (isMin ? 1 : 0);
There was a problem hiding this comment.
7adde4c で bool 型のローカル変数を当該関数から追放しました。
int2dec 関数の単体テストを記述
There was a problem hiding this comment.
いっぱい対応ありがとうございます。
可能であれば、現バージョンで再度プロファイルを取って、貼り付けていただけますか?
#418 (comment)
|
レビューありがとうございました。Merge します。 |
CGrepAgent::SetGrepResult 出力形式がノーマルの場合の処理の高速化
Grep結果を構築する
CGrepAgent::SetGrepResultメソッドにおいて出力形式がノーマルの場合に、マッチした行番号と桁番号を sprintf で文字列化する処理がありますが、プロファイラで確認したところその処理に少し時間が掛かっている事が分かりました。その文字列化する処理を別途用意する事で高速化しました。場合によるかもしれませんが、Grep処理が 5% くらい速くなりました。