Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSVダウンロード及びグラフ表示が完了しない #51

Open
zenith6 opened this issue May 18, 2016 · 20 comments
Open

CSVダウンロード及びグラフ表示が完了しない #51

zenith6 opened this issue May 18, 2016 · 20 comments

Comments

@zenith6
Copy link
Contributor

zenith6 commented May 18, 2016

DBに問い合わせデータが大量に溜まっている状態で、CSVの全件ダウンロードやグラフを表示しようとすると、サーバーから応答待ちのまま接続が切断される等の現象が起き、正常に完了しません。

PHP の設定の memory_limit を増やした場合、リクエストが正常に完了します。

よってメモリ制限に引っかかっているのではないかと推測されます。

@inc2734
Copy link
Owner

inc2734 commented May 22, 2016

いただいたプルリクエストではwp_cache_flush()で都度メモリキャッシュをクリアすることで解決を図られていましたが、投稿を取得する際にcache_results => falseとすることで「そもそもキャッシュしない」のほうがスマートなのでは?とも思ったのですがいかがでしょう?
https://developer.wordpress.org/reference/classes/wp_query/#source-code

wp_cache_flush()だと意図していないキャッシュまでクリアして何か問題が起こることがないのかなぁということも思いまして…。

@zenith6
Copy link
Contributor Author

zenith6 commented May 22, 2016

投稿を取得する際にcache_results => falseとすることで「そもそもキャッシュしない」のほうがスマートなのでは?とも思ったのですがいかがでしょう?

その通りですね!
キャッシュして欲しいものまで破棄していたので非効率だと気になっていました。
とても良い解決方法だと思います。

@inc2734
Copy link
Owner

inc2734 commented May 22, 2016

なるほど!
#52 はマージしたのですが、ひとまず一旦 revert します、すみません。
もしcache_results => falseでもメモリ不足が解決しないようであればまたマージさせていただきます!

@zenith6
Copy link
Contributor Author

zenith6 commented May 22, 2016

かしこまりました!
私の方でも効果を確認してご報告申し上げます。

それと #53 のコミットは今から rebase します。
'cache_results' = false + #52 でリファクタリングして頂いたスタイルに近づけます。

@inc2734
Copy link
Owner

inc2734 commented May 22, 2016

ありがとうございます!!

@zenith6
Copy link
Contributor Author

zenith6 commented May 24, 2016

キャッシュの無効化に 'cache_results' = false を使ってみようの件の、結果のご報告です。
結論から申し上げると利用できないパターンが見付かりました><

https://github.com/WordPress/WordPress/blob/4.5.2/wp-includes/query.php#L3586-L3608
前後で $split_the_querytrue になると _prime_post_caches() が呼び出され、
cache_results や update_post_term_cache 等の値に関係なく投稿データがキャッシュされる、
という流れです。
https://github.com/WordPress/WordPress/blob/4.5.2/wp-includes/post.php#L5694

お手間でなければ検証にご協力頂けましたら助かります!

@inc2734
Copy link
Owner

inc2734 commented May 26, 2016

なるほど…。情報ありがとうございます!split_the_query にはフィルタがあるようですが、ここを変えるほうがキャッシュをクリアするより怖い感じがしますね(調べてはいないので直感ですが)。それであれば'cache_results' = falseより頂いたプルリクの処理を適用したほうが良さそうですね。

ちなみに、検証はどのように行われていますでしょうか?
大量の問い合わせデータを登録する簡単な方法を何かご存知であれば教えていただきたいなと思いまして…。

@zenith6
Copy link
Contributor Author

zenith6 commented May 26, 2016

split_the_query にはフィルタがあるようですが、ここを変えるほうがキャッシュをクリアするより怖い感じがしますね(調べてはいないので直感ですが)。

そうかフィルターで書き換えられますね!
出来れば対応したいのですが時間が取れなくなってしまいました…
この件は余力のある方にお願いしたいです!

ちなみに、検証はどのように行われていますでしょうか?
大量の問い合わせデータを登録する簡単な方法を何かご存知であれば教えていただきたいなと思いまして…。

wp_insert_post()update_post_meta() で直接入れてます。
DBに登録するコードを追いかけていたのですが途中で挫折しました^^

add_action( 'init', function () {
    $form_key = 1234;
    $q = 10000;

    for ( $i = 1; $i <= $q; $i ++ ) {
        $post_id = wp_insert_post(
            array(
                'post_type'    => 'mwf_' . $form_key,
                'post_content' => '',
                'post_title'   => $i,
                'post_status'  => 'publish',
            ) );

        update_post_meta( $post_id, 'A', mt_rand() );
        !( $i % 2 ) && update_post_meta( $post_id, 'B', mt_rand() );
        !( $i % 3 ) && update_post_meta( $post_id, 'C', mt_rand() );
        !( $i % 4 ) && update_post_meta( $post_id, 'D', mt_rand() );
        !( $i % 5 ) && update_post_meta( $post_id, 'E', mt_rand() );
    }
} );

@inc2734
Copy link
Owner

inc2734 commented May 26, 2016

wp_insert_post() と update_post_meta() で直接入れてます。

コードありがとうございます!これで僕もテストしてみます。
僕も仕事が忙しくなってきていてなかなか調査できずにおりますが、必ず検証して取り込みますのでもう少々お待ち頂ければと思います!

@zenith6
Copy link
Contributor Author

zenith6 commented May 27, 2016

ありがとうございます!正座して待ってます。
お忙しい折に本当に申し訳ないですが、よろしくお願い申し上げます。
私も時間が取れ次第ご協力致します。

あと、#53#52 のリファクタリング内容に合わせて書き直しますと言っていた件は、
88ec314posts_per_page に関する懸念があるため、保留にしておきます。

@inc2734
Copy link
Owner

inc2734 commented May 28, 2016

とりあえず使用メモリの調査だけ行いました。#54
結論からいうと、'cache_results' = false でも split_the_filter = false でもメモリ使用量は減りませんでした(むしろ増えて Fatal Error となりました)。
従いまして 88ec314posts_per_page に問題があれば修正したバージョンが正式版となりそうです。取り急ぎご報告まで。

@zenith6
Copy link
Contributor Author

zenith6 commented May 28, 2016

ご連絡ありがとうございます!
キャッシュ周りの挙動は一筋縄にはいかないのですね。残念です。

@inc2734
Copy link
Owner

inc2734 commented May 28, 2016

その後挙動を確認していたのですが、どうもプルリクいただいた 1a767f2 が正しく動作していないような気がします。約11,000件のデータを登録して全件ダウンロードしたところ、なぜか約3,000件のデータしかダウンロードされませんでした(メモリ使用量約111MB)。

次のように修正したところ全件ダウンロードできました(メモリ使用量約230MB)。
1a767f2...issues/#51

@zenith6
Copy link
Contributor Author

zenith6 commented May 28, 2016

なぜか約3,000件のデータしかダウンロードされませんでした

ご指摘の通り私がやらかしていました…
申し訳ございません><

230MBはPRなしの状態から変わってないという事でしょうか?

@inc2734
Copy link
Owner

inc2734 commented May 28, 2016

230MBはPRなしの状態から変わってないという事でしょうか?

プルリク無し(v2.8.2)の段階だと約220MBになるので逆にちょっと増えてますね^^;

@zenith6
Copy link
Contributor Author

zenith6 commented May 28, 2016

orz...
手元で調べ直します。

@zenith6
Copy link
Contributor Author

zenith6 commented May 28, 2016

手元では改善がみられるような差が出ました。

_ 開始時 終了時 増減 開始時ピーク 終了時ピーク ピーク増減
master 92f1dac 43333704 165735592 +122401888 43717976 233824128 +190106152
issues/#51 02399e8 43331528 43069240 -262288 43710728 44156656 +445928
#52 + fix zenith6/mw-wp-form@e155e5c 43328392 43065656 -262736 43705296 44133008 +427712

計測は以下のコードで。

class MW_WP_Form_CSV {
    public function download() {
        $mem_start      = memory_get_usage();
        $mem_peak_start = memory_get_peak_usage();

// ダウンロード処理

        $mem_end        = memory_get_usage();
        $mem_peak_end   = memory_get_peak_usage();
        $mem_delta      = $mem_end - $mem_start;
        $mem_peak_delta = $mem_peak_end - $mem_peak_start;

        exit; // break point
    }
}

@inc2734
Copy link
Owner

inc2734 commented May 28, 2016

なるほど!ありがとうございます。僕の計測方法がおかしかったみたいですね。ご提示いただいた方法で、僕の方でも再度計測してみます。ありがとうございます!

@inc2734
Copy link
Owner

inc2734 commented Jun 6, 2016

とりあえず最新の修正版だけテストしましたが、明らかに2人の結果に違いがあるように見えます。僕が何か間違っているのでしょうか…。#54

@zenith6
Copy link
Contributor Author

zenith6 commented Jun 12, 2016

返答が遅れて大変失礼致しました!

再現性を共有できないのが問題ですね。
計測コードやら手順やら実行環境やらPRを投げた私が用意すべきですが、
本格的に忙しくなり時間を取れそうにないです…。
申し訳ございませんが、しばらくお待ち頂いてよろしいでしょうか><

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants