Hello On 2019/09/06 23:19, Tomas Vondra wrote: > On Wed, Sep 04, 2019 at 07:19:47PM +0300, Sergei Kornilov wrote: >> >> ... >> >> Results: >> >> test | mode | average_tps | degradation_perc >> ----------------------+----------+-------------+------------------ >> head_no_pgss | extended | 13816 | 1.000 >> patch_not_loaded | extended | 13755 | 0.996 >> head_track_none | extended | 13607 | 0.985 >> patch_track_none | extended | 13560 | 0.981 >> head_track_top | extended | 13277 | 0.961 >> patch_track_top | extended | 13189 | 0.955 >> patch_track_planning | extended | 12983 | 0.940 >> head_no_pgss | prepared | 29101 | 1.000 >> head_track_none | prepared | 28510 | 0.980 >> patch_track_none | prepared | 28481 | 0.979 >> patch_not_loaded | prepared | 28382 | 0.975 >> patch_track_planning | prepared | 28046 | 0.964 >> head_track_top | prepared | 28035 | 0.963 >> patch_track_top | prepared | 27973 | 0.961 >> head_no_pgss | simple | 16733 | 1.000 >> patch_not_loaded | simple | 16552 | 0.989 >> head_track_none | simple | 16452 | 0.983 >> patch_track_none | simple | 16365 | 0.978 >> head_track_top | simple | 15867 | 0.948 >> patch_track_top | simple | 15820 | 0.945 >> patch_track_planning | simple | 15739 | 0.941 >> >> So I found slight slowdown with track_planning = off compared to HEAD. >> Possibly just at the level of measurement error. I think this is ok. >> track_planning = on also has no dramatic impact. In my opinion >> proposed design with pgss_store call is acceptable. >> > > FWIW I've done some benchmarking on this too, with a single pgbench client > running select-only test on a tiny database, in different modes (simple, > extended, prepared). I've done that on two systems with different CPUs > (spreadsheet with results attached).
Refering to Sergei's results, if a user, currently using pgss with tracking execute time, uses the new feature, a user will see 0~2.2% performance regression as below. >> head_track_top | extended | 13277 | 0.961 >> patch_track_planning | extended | 12983 | 0.940 >> patch_track_planning | prepared | 28046 | 0.964 >> head_track_top | prepared | 28035 | 0.963 >> head_track_top | simple | 15867 | 0.948 >> patch_track_planning | simple | 15739 | 0.941 If a user will not turn on the track_planning, a user will see 0.2-0.6% performance regression as below. >> head_track_top | extended | 13277 | 0.961 >> patch_track_top | extended | 13189 | 0.955 >> head_track_top | prepared | 28035 | 0.963 >> patch_track_top | prepared | 27973 | 0.961 >> head_track_top | simple | 15867 | 0.948 >> patch_track_top | simple | 15820 | 0.945 > > I don't see any performance regression - there are some small variations > in both directions (say, ~1%) but that's well within the noise. So I think > the patch is fine in this regard. +1 I also saw the codes and have one comment. [0002 patch] In pgss_planner_hook: + /* calc differences of buffer counters. */ + bufusage = compute_buffer_counters(bufusage_start, pgBufferUsage); + + /* + * we only store planning duration, query text has been initialized + * during previous pgss_post_parse_analyze as it not available inside + * pgss_planner_hook. + */ + pgss_store(query_text, Do we need to calculate bufusage in here? We only store planning duration in the following pgss_store. -- Yoshikazu Imai
