[PATCH][LNT] Add ability to filter machines on daily report page + refactor LNT regression tests to enable properly testing it.

Mon Jun 8 02:21:09 PDT 2015

Right, so it seems that having both server-side filtering and client-side filtering is useful.

The attached patches implement that:

0001-Add-*: adds the server-side filtering for both the webui and the email daily report.

0002-Factor-out-*: refactors the regex functionality that Chris added for the run and machines pages, so the code isn’t repeated and easier to reuse. NFC.

0003-Run-page-*: makes sure that on the run page, only machine names are tested against the regex, not the data in the other columns in the table.

0004-Add-regex-*: adds the client-side regex filtering for machine names on the daily report page.

These patches implement a consistent interface for server-side daily report filtering on machine names, both for the webui and the daily report.

The client-side regex filtering is factored out, so having a consistent look-and-feel for the client-side filtering is maintained.

If useful, on the daily report page, a search box for filtering based on benchmark name could also be added easily after the above patches have been committed.

Does this look OK?

Thanks,

Kristof

From: Chris Matthews [mailto:chris.matthews at apple.com] 
Sent: 04 June 2015 21:11
To: Kristof Beyls
Cc: James Molloy; llvm-commits
Subject: Re: [PATCH][LNT] Add ability to filter machines on daily report page + refactor LNT regression tests to enable properly testing it.

I added some simple regex filters to the run and machines page.  They would work for this purpose too.

Server side filtering has the advantage of limiting the query size, which is super useful for me. Our 7 day reports runs on enough machines that it is slow (~100k samples returned by the query).  Being able to limit it would be handy.

On Jun 4, 2015, at 7:04 AM, Kristof Beyls <kristof.beyls at arm.com> wrote:

Yeah – that’s another possibility.

I do like keeping the functionality that with the URL you can specify which machines to see by default, e.g.
allowing to bookmark your favourite machines.
I’m assuming it’s easy to access URL query parameters from javascript. If so, the query parameter could
control which checkboxes are enabled by default.

I think the biggest disadvantage of doing it with javascript is that the interface becomes inconsistent
with the “lnt send-daily-report” command, as that command obviously cannot run javascript regexes.

I haven’t checked in detail, but there probably are differences between javascript regexes and python regexes.

Another potential advantage of doing the filtering server-side is that the size of the daily report
page could be reduced.  However, I haven’t seen examples of huge daily report pages so far, so
I think this is currently just a theoretical advantage.

Overall, I like your suggestion of checkboxes and enabling interactive filtering – but it would be nice
to find some kind of solution to keep the interface between the daily report webui and the daily report
emailing command consistent. Maybe the differences between javascript regexes and python regexes
won’t be much of a problem in practice?

Thanks,

Kristof

From: James Molloy [mailto:james at jamesmolloy.co.uk] 
Sent: 04 June 2015 13:51
To: Kristof Beyls; Chris Matthews
Cc: llvm-commits
Subject: Re: [PATCH][LNT] Add ability to filter machines on daily report page + refactor LNT regression tests to enable properly testing it.

Hi Kristof,

I was thinking about this today, and I think the easiest way to work this from a UI perspective would be to just provide checkboxes next to the machine in the overview table - toggling these boxes would toggle the visibility of results from that run on-the-fly (via javascript), similar to how the new test regexp filter box works in the v4_runs page.

What do you think?

Cheers,

James

On Thu, 4 Jun 2015 at 12:09 Kristof Beyls <kristof.beyls at arm.com> wrote:

Thanks for the review, I've committed the test refactorings as
r238296-r238300. You're right, it makes sense to filter machines
based on a pattern rather than on the full name. I've considered
just a sub-string search, matching with a glob or matching with
a python regex. I feel that matching with a python regex is the
best solution as it allows the most flexible filtering.

I went for "?filter-machine-regex=" to enable filtering on the
daily report webui and "--filter_machine_regex" to enable
filtering when using the lnt send-daily-report command.

See attached patch for details. Does this look OK to you?

Kristof

> -----Original Message-----
> From: Chris Matthews [mailto:chris.matthews at apple.com]
> Sent: 26 May 2015 19:28
> To: Kristof Beyls
> Cc: llvm-commits
> Subject: Re: [PATCH][LNT] Add ability to filter machines on daily report
> page + refactor LNT regression tests to enable properly testing it.
>
> I completely agree with this testing strategy. This is a much better way
> to test.
>
> I'm fine with an explicit machine list, but I wonder if a more useful
> way to do this is to have a machine name filter? That way queries like
> "?filter-machine=AArch64&" could be possible along with an explicit
> "?filter-machine=lvm-juno-lnt-perf__LNT-AArch64-A53-
> O3__clang_DEV__aarch64&".  Just substring matching on the machine name.
> That would make it very easy to grab classes of machines.
>
> Please add period to end of comment on patch 6 line 46.
>
> Could you update the command line daily report tool
> (lnt/lnttool/main.py:304) to also have this functioanltiy? I think the
> change should be really simple.
>
> Otherwise looks good!
>
> > On May 26, 2015, at 3:41 AM, Kristof Beyls <kristof.beyls at arm.com>
> wrote:
> >
> > Hi Chris,
> >
> > I'd like to be able to filter on the daily report page, e.g.
> > http://llvm.org/perf/db_default/v4/nts/daily_report/2015/5/21?num_days
> > =7, so that it only shows data for a subset of the builders. The
> > previous link shows many results coming from the penryn builders,
> > which make it hard to just investigate the results coming from the
> > juno board.
> >
> > 0006-Enable-filtering-of-machines-shown-on-LNT-daily-repo.patch adds
> > support for this; the user visible behaviour I went for is adding
> > machines={comma-separated list of machine names} to the query string
> > of the url, e.g.
> > http://llvm.org/perf/db_default/v4/nts/daily_report/2015/5/21?num_days
> > =7&mac
> > hines=llvm-juno-lnt-perf__LNT-AArch64-A53-O3__clang_DEV__aarch64.
> >
> > To be able to add a proper regression test/unit test for this, I also
> > had to refactor the LNT regression tests a bit, which is what the
> > other patches contain. In a number of steps, they transform the tests
> > using tests/SharedInputs/SmallInstance/lnt.db to using a database
> > constructed from human-readable SQL statements, creating the sqlite
> > file on the fly during test execution. This enables adding per-test
> > records to the database:
> >
> > - 0001-For-the-regression-tests-make-temporary-LNT-db-insta.patch
> > - 0002-Replace-binary-test-lnt.db-with-human-readable-text-.patch
> > - 0003-replace-hexadecimal-encoding-of-blobs-with-textual-r.patch
> > - 0004-remove-most-of-the-unnecessary-insert-statement-from.patch
> > - 0005-Enable-adding-test-specific-records-to-test-database.patch
> >
> > My hope is that this refactoring enables creating new functionality
> > for the LNT ui in a more Test-Driven Development way, which should
> > make it easier for more people to contribute to LNT & to reduce the
> > likelihood of regressions in LNT.
> >
> > What do you think?
> > Is the refactoring of the regression tests a step in the right
> direction?
> > Any objections to adding the machine= filter to the daily report page?
> >
> > Thanks,
> >
> > Kristof
> > <0001-For-the-regression-tests-make-temporary-LNT-db-insta.patch><0002
> > -Replace-binary-test-lnt.db-with-human-readable-text-.patch><0003-repl
> > ace-hexadecimal-encoding-of-blobs-with-textual-r.patch><0004-remove-mo
> > st-of-the-unnecessary-insert-statement-from.patch><0005-Enable-adding-
> > test-specific-records-to-test-database.patch><0006-Enable-filtering-of
> > -machines-shown-on-LNT-daily-repo.patch>

_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150608/f0be640e/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-filter-machine-regex-functionality-to-daily-repo.patch
Type: application/octet-stream
Size: 13448 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150608/f0be640e/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Factor-out-common-javascript-regex-search-code-simpl.patch
Type: application/octet-stream
Size: 4693 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150608/f0be640e/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Run-page-regex-search-only-search-for-matches-with-b.patch
Type: application/octet-stream
Size: 2497 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150608/f0be640e/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Add-regex-filter-box-for-machine-names-to-daily-repo.patch
Type: application/octet-stream
Size: 3574 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150608/f0be640e/attachment-0003.obj>