[DAO-118] 検索結果をMappingする時にMappingに無関係のPropertyを除外し、パフォーマンスを改善しました。 Created: 2007-08-24  Updated: 2007-09-11  Resolved: 2007-09-01

Status: Resolved
Project: S2Dao
Component/s: None
Affects Version/s: None
Fix Version/s: 1.0.47-RC1

Type: Improvement Priority: Major
Reporter: jflute Assignee: jflute
Resolution: Fixed Votes: 0
Labels: None


 Description   

S2Daoのソースを色々見ていたところ、

AbstractBeanMetaDataResultSetHandler#createRow()において、
以下の箇所が、Mappingとは無関係のPropertyのときも評価されており、
Entityの作り方次第で、同じSQLでも大量件数取得時(100件くらいでも)にSpeedに差が出ます。

*自テーブルのMappingの話に限定しています。

} else if (!pt.isPersistent()) {
    for (Iterator iter = columnNames.iterator(); iter.hasNext();) {
        String columnName = (String) iter.next();
        String columnName2 = StringUtil
                .replace(columnName, "_", "");
        if (columnName2.equalsIgnoreCase(pt.getColumnName())) {
            ValueType valueType = pt.getValueType();
            Object value = valueType.getValue(rs, columnName);
            PropertyDesc pd = pt.getPropertyDesc();
            pd.setValue(row, value);
            break;
        }
    }
}

Mappingとは無関係のPropertyとは、
A. Setterの無いGetter
B. 子Tableの持つなどMappingとは別のTiming設定するjava.util.ListのPropertyなど

「A」は明らかです(Setできないし)。
「B」は「じゃあ、Mapはどうなんだ!?」とか線引きが難しいです。

そこで以下の修正案です。

1. 上記の処理の中でpt.getPropertyDesc().isWritable()でないものはすぐにcontinueする。

→ 「A」のみ対応

2. FastPropertyTypeFactoryとPropertyTypeFactoryImplにおいてpt.getPropertyDesc().isWritable()でないものは対象外とする。

(BeanMetaDataの属性として保持しないということ)
→ 「A」のみ対応
※RelationのMapping時にも有効になって一石二鳥。
※BeanMetaDataでReadOnlyのPropertyを保持しないでOKかどうかちょっとパッとわかりませんでした。

3. 上記の処理において、1ループ目でMapping対象となる「pt」の別途Listに詰めて2ループ目以降は、それを使う。

→ 「A」も「B」も対応
※createRow()の引数が一つ増えるので、少し影響範囲大きい。

4. 「2」 and 「3」の対応をする。

→ 「2」を対応するにしても「3」 をやるメリットはある。逆もまたしかり。

5. 何もしない

→ 実際に手動でEntityを作っていればあまり、そういったPropertyを作らないかもしれない。
DBFluteだと、Setterの無いGetterをEntityに作ることがあるので影響があるのだが、
それはDBFluteでどうにか対応すればよい。
(拡張のために上記の処理部分を別クラスで差し替え可能にするくらいはしたい....)

現在S2Daoの利用者はかなり多いため、慎重に修正方針を決めたいと考えます。
皆様、どう思われますでしょうか?ご意見下さい。



 Comments   
Comment by jflute [ 2007-09-01 ]

StatusをResolveにするのを忘れていました。
Relsolveにします。

Comment by jflute [ 2007-08-26 ]

DTOも対応しました。
もともとDTOの方はそんなに遅くなかったですが、
統一性の観点から同じ構造にしました。
(1000件HIT検索で平均160msが平均140msくらいにはなったかな...)

Comment by jflute [ 2007-08-26 ]

Relationも対応しました。

Entityの作り方次第で変動するので正確ではないですが(かつローカル環境)、
S2Dao-1.0.46から比べて、単純な1000件HIT検索で平均400msが平均150msになりました。すごい。

Comment by jflute [ 2007-08-26 ]

補足ですが:

Map cache = null;
while(rs.next()) {
  if (cache == null) {
    cache = createCache(...);
  }
  Object row = createRow(..., cache);
  ...
}

と言う感じにしました。
検索結果が1件も存在しない場合には
Cacheを作成しないようにしています。

Comment by jflute [ 2007-08-26 ]

非常に良いと思います。
そこまで変えてしまっても良いかなとちょっと思いましたが、
ここまできたらやってしまいましょう。
(DBFlute側のテストもあることですし)

対応しました。
S2Daoのテスト/DBFluteのテスト両方グリーンです。
しかも、ちょっと正確に測ってないですが、1000件検索随分速くなった気がします。
(少なくとも10msは速くなってる)

Relationの方はもう少し複雑なのですが、多分いけると思います。
ちょっと検討してみます。

Comment by taedium [ 2007-08-26 ]

RowCreatorImplですが、
キャッシュを使う場合でもsetupPropertyメソッド以降で
条件分岐やループが行われていますが、
実はこのあたり一回だけ動けばいいように見えました。

PropertyTypeのキャッシュはSetじゃなくてkeyをカラム名にした
Mapにして、キャッシュの作成とrowの作成を完全に別処理にして
しまうのはどうでしょう。

イメージとしてはこんな感じを思い浮かべてみました。

 
Map cache = createCache(...); // 呼び出し先でsetupPropertyメソッド相当の処理
while(rs.next()) {
  Object row = createRow(..., cache);
  ...
}

RelationRowCreatorImplの方はちょっと確信ないですが、
同じような対応で効果があるように見えました。

Comment by jflute [ 2007-08-25 ]

Relationの方も全く同じ方法で対応しました。

1000件検索で、
Mappingに関係の無いPropertyを6個持つEntityが戻り値において 平均500ms → 平均200ms になりました。
Relationにも対応したことにより、素のEntityとのSpeedの差はほとんどなくなりました。
(差はあっても数ミリ秒。別の要因で簡単に覆る程度なので無いと考えて良いかと)

特にバッチでの検索などでかなり向上したのではないかと思います。
ご意見ありがとうございました。

こちらC#版でも全く同じですので対応を検討したいと思います。
(C#の方のJIRAへ同様の課題にしておきます)

Comment by jflute [ 2007-08-25 ]

Relationの方も同じやり方でできないか探ってみます。

Comment by jflute [ 2007-08-25 ]

「1」と「3」をローカルで実装してみました。
(「1」も他の影響が無いため&1レコード目のPerformanceが良くなる)

S2Dao/DBFluteのUTは全て通りました。
問題なく値は取得できているようです。
また、DBFluteのPerformance測定のTestにおいても
ほとんど、差が無くなりました。
(Mappingとは無関係のPropertyが定義されていても速い)

他に何か意見がありましたら、お願いします。
(少し精査して特に問題なければこれでいきたいと思います)

Comment by jflute [ 2007-08-24 ]

> 「1」と「2」の場合ですが、PropertyDesc()#isWritable()はS2.4のメソッドなのでS2.3では
> PropertyDesc()#hasWriteMethod()を呼ぶようにする必要がありますね。

おっと、なるほど。気をつけなければならないですね。
ありがとうございます。

> 影響範囲を考えると「3」

なるほど、逆に「3」の方が限定的で安全というところですね。

Comment by taedium [ 2007-08-24 ]

あっ、上のコメントしたのは私(taedium)です。

Comment by Anonymous [ 2007-08-24 ]

影響範囲を考えると「3」が安全だと思います。
「1」と「2」の場合ですが、PropertyDesc()#isWritable()はS2.4のメソッドなのでS2.3では
PropertyDesc()#hasWriteMethod()を呼ぶようにする必要がありますね。

Comment by jflute [ 2007-08-24 ]

うおお、キレイになった!ありがとうございます!
JIRAの書き方ってのがあるんですね。。。

Comment by manhole [ 2007-08-24 ]

カラム名とプロパティ名のマッチングは、結果セットが1000行あっても1度だけ行えば良いのですから、ループの初回のみでマッチングをすれば良いのかなあと思います。
なので「3」ですかねぇ。

Comment by manhole [ 2007-08-24 ]

もとの投稿をJIRAっぽく整形してみました。

Comment by jflute [ 2007-08-24 ]

個人的には
(影響の大きさとか)色々考えてて、
現状では「2」と「5」ってのもアリかと思っています。
つまり、S2Daoとして「2」のみ対応。

Comment by jflute [ 2007-08-24 ]

「2」に関してですが、
AbstractPropertyTypeFactory#createDtoPropertyTypes()も
修正対象ですね。。。

Comment by jflute [ 2007-08-24 ]

「2」を試しにローカルで修正してもS2DaoのUTは全部通りますね。
DBFluteのUTも2つ除いて通りました。これだけでもかなり速くなりました。
(落ちた2つはpropertyTypeFactoryBuilderが無いって
怒られた...別問題ですね)

Generated at Tue Apr 16 21:15:33 JST 2024 using Jira 9.15.0#9150000-sha1:9ead8528714127d8cfabf2446010d7e62c0a195c.