RE: Scaling Ceph reviews and testing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Podoski, Igor
> Sent: Thursday, November 26, 2015 10:25 AM
> 
> > But correctness and reliability regressions are one thing, performance
> > regressions are another one. I already see PRs that promise
> > performance increase, when at (my) first glance it looks totally
> > contradictory, or it's just a big, 100+ line change which adds more
> > complexity than performance. Not to mention utter nonsense like
> > https://github.com/ceph/ceph/pull/6582
> > (excuse my finger-pointing, but this case is so extreme that it needs
> > to be pointed out). Or, to put it more bluntly, some folks are
> > spamming with performance PRs that in their opinion improve something,
> > while in reality those PRs at best increase complexity of already
> > complex code and add (sometimes potential) bugs, often with added
> > bonus of actually degraded performance.
> > [..]
> 
> We could also add another label, like "explanation/data needed" and guys
> marking new PR's could add this to make this more restrict:  "Performance
> enhancements must come with test data and detailed explanations."
> (https://github.com/ceph/ceph/blob/master/CONTRIBUTING.rst )

There's already "pending-discussion" tag which could be used too.

> Then Piotr's idea will be easier to do, when "PR validator" will have test data
> and explanation he could faster/easier decide if this PR make sense or not.

Actually, it seems to be doable. PR submitter could submit some code that triggers optimized code path and some bot could build the PR branch and compare some performance metrics before and after applying modified code. The catch here is that it needs to be done on bare-metal machine, VMs are often too unstable in that regard and might give totally different results each run.


With best regards / Pozdrawiam
Piotr Dałek

��.n��������+%������w��{.n����z��u���ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f




[Index of Archives]     [CEPH Users]     [Ceph Large]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux