Hi Ken, just trying to review these changes - I guess the crux
of the issue falls on the "fixed logic errors elsewhere", as mentioned :
'With hindsight, it appears the guards were an incorrect attempt
to address logic errors elsewhere, and when these other errors
were finally all fixed properly, the guards were no longer needed
for correctness, but they did sometimes trigger lots of archive
reading that was not required.'
So whilst I can review the code change involving the removal of
the guard (which looks good at the surface), no idea what other fixes
were made to remove the need for that guard. I think we can just
take your word for it!
I built and installed with these changes and checked qa/787 passes.
But I've lost Frank and Paul's recipe for reproducing the archive replay
performance issue - anyone have a reference to that handy?
Regards
-- Mark
On 04/23/2015 09:30 AM, Ken McDonell wrote:
Paul and Frank ... I think this finally (sigh) nails your PCP archive poor
performance problems.
Changes committed to git://git.pcp.io/kenj/pcp master
Ken McDonell (4):
libpcp/derive.c: fix typo in diagnostic message
qa/admin/check-vm: Add note to ignore networking warnings if not running
QA
libpcp/interp.c: another performance improvement
libpcp/interp.c: major breakthrough!
qa/180.out.3 | 2 +-
qa/787.out | 8 ++++----
qa/admin/check-vm | 11 +++++++----
src/libpcp/src/derive.c | 2 +-
src/libpcp/src/interp.c | 48
+++++++++++++++++++++++++++---------------------
5 files changed, 40 insertions(+), 31 deletions(-)
Details ...
commit 2a3b5ff6668c13791e9c9df64673419dd7903281
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Thu Apr 23 09:10:07 2015 +1000
libpcp/interp.c: major breakthrough!
One and off for more than 15 months(!), I've been trying to
understand two guards that control the backwards and forwards
searching when trying to "bound" a metric-instance for a point in
time value interpolation from a PCP archive. This is in the dark
heart of some of the most complex code in libpcp, and sometimes
was observed to trigger obscene amounts of archive record reading,
and re-reading, and re-reading ...
In the guards one clause in particular made no sense to me ... but
each time I tried to remove these clauses or refine them so they
made less nonsense, there were a spate of hard to understand and
harder to diagnose QA failures.
After many other changes to improve the logic in the code to
deal with unrelated corner cases, I finally was down to a single
QA anomaly. In desperation, I removed the "makes no sense to me
guards" and bingo!
All tests pass, and the performance problems reported by Frank
(in a pmmgr context) and Paul Smith (in a pmchart context with big
production archives) have both gone away _and_ interp mode seems
to produce the correct metric values across all of the QA tests.
With hindsight, it appears the guards were an incorrect attempt
to address logic errors elsewhere, and when these other errors
were finally all fixed properly, the guards were no longer needed
for correctness, but they did sometimes trigger lots of archive
reading that was not required.
For the historical record the guard that has been dropped is this one
(for forward search and the similar one for backwards search):
/* ...
* t_next is a mark and t_next > t_req => need to search
* back also (unless we've already scanned to this mark)
...
*/
...
(IS_MARK(icp->s_next) && !IS_SCANNED(icp->s_next) && icp->t_next >
t_req) ||
Now back to the rest of my life!
commit 8a28abe2c3ecd75daef3e01424a64523d8601738
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Thu Apr 23 08:31:12 2015 +1000
libpcp/interp.c: another performance improvement
Small one this time, we're down to the obscure corner cases now.
commit b113884be5060a1a99ddbef94baaab416638bbca
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Thu Apr 23 08:29:17 2015 +1000
qa/admin/check-vm: Add note to ignore networking warnings if not running QA
commit 81d987bf77a8f74d29847fd136fb903bee09c8d4
Author: Ken McDonell <kenj@xxxxxxxxxxxxxxxx>
Date: Thu Apr 23 08:28:40 2015 +1000
libpcp/derive.c: fix typo in diagnostic message
_______________________________________________
pcp mailing list
pcp@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/pcp
|