| Message ID | 1474306544-24708-2-git-send-email-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 09/19/2016 12:35 PM, Daniel P. Berrange wrote: > The 'check' program records timings for each test that > is run. These timings are only valid, however, for a > particular format/protocol combination. So if frequently > running 'check' with a variety of different formats or > protocols, the times printed can be very misleading. > > Record the protocol/format in the check.time file and > throw it away if it doesn't mach the current run args. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > tests/qemu-iotests/check | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) Rather than completely throwing things away, would it be worth updating the check.time file format to track multiple entries? For every distinct args seen, track a timing for that combination of args, then when starting a test, if a line in the file contains the current args, report that old time; if not, then append a line with the new args. The file grows according to how many distinct args combinations you use, and it's probably easier to make 'a b' and 'b a' report as different timings even if they have the same effect and could share a timing. We'd also want an operation to clean out timings without running tests, particularly if timings can otherwise grow huge due to every possible args combination. What do you think?
On Mon, Sep 19, 2016 at 02:53:36PM -0500, Eric Blake wrote: > On 09/19/2016 12:35 PM, Daniel P. Berrange wrote: > > The 'check' program records timings for each test that > > is run. These timings are only valid, however, for a > > particular format/protocol combination. So if frequently > > running 'check' with a variety of different formats or > > protocols, the times printed can be very misleading. > > > > Record the protocol/format in the check.time file and > > throw it away if it doesn't mach the current run args. > > > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > > --- > > tests/qemu-iotests/check | 21 +++++++++++++++++++-- > > 1 file changed, 19 insertions(+), 2 deletions(-) > > Rather than completely throwing things away, would it be worth updating > the check.time file format to track multiple entries? For every distinct > args seen, track a timing for that combination of args, then when > starting a test, if a line in the file contains the current args, report > that old time; if not, then append a line with the new args. The file > grows according to how many distinct args combinations you use, and it's > probably easier to make 'a b' and 'b a' report as different timings even > if they have the same effect and could share a timing. We'd also want > an operation to clean out timings without running tests, particularly if > timings can otherwise grow huge due to every possible args combination. > > What do you think? I was afraid someone would suggest a more complex scheme like that :-) I guess we could keep things simple by not inventing a new format, but instead of using 'check.time', use 'check.time.$FORMAT-$PROTOCOL' eg 'check.time.qcow2-file' Regards, Daniel
On 09/20/2016 04:36 AM, Daniel P. Berrange wrote: >> Rather than completely throwing things away, would it be worth updating >> the check.time file format to track multiple entries? For every distinct >> args seen, track a timing for that combination of args, then when >> starting a test, if a line in the file contains the current args, report >> that old time; if not, then append a line with the new args. The file >> grows according to how many distinct args combinations you use, and it's >> probably easier to make 'a b' and 'b a' report as different timings even >> if they have the same effect and could share a timing. We'd also want >> an operation to clean out timings without running tests, particularly if >> timings can otherwise grow huge due to every possible args combination. >> >> What do you think? > > I was afraid someone would suggest a more complex scheme like that :-) > > I guess we could keep things simple by not inventing a new format, > but instead of using 'check.time', use 'check.time.$FORMAT-$PROTOCOL' > eg 'check.time.qcow2-file' Seems more palatable. Lots of files rather than lots of lines in a single file; searching is now fast (if you can come up with the right file name, the OS does the searching for you rather than us having to figure out which (if any) portion of a large file applies). We may still want a command for easily cleaning everything, and if we do switch to new file names, we'll still want to clean up the old files.
On Tue, Sep 20, 2016 at 09:06:17AM -0500, Eric Blake wrote: > On 09/20/2016 04:36 AM, Daniel P. Berrange wrote: > >> Rather than completely throwing things away, would it be worth updating > >> the check.time file format to track multiple entries? For every distinct > >> args seen, track a timing for that combination of args, then when > >> starting a test, if a line in the file contains the current args, report > >> that old time; if not, then append a line with the new args. The file > >> grows according to how many distinct args combinations you use, and it's > >> probably easier to make 'a b' and 'b a' report as different timings even > >> if they have the same effect and could share a timing. We'd also want > >> an operation to clean out timings without running tests, particularly if > >> timings can otherwise grow huge due to every possible args combination. > >> > >> What do you think? > > > > I was afraid someone would suggest a more complex scheme like that :-) > > > > I guess we could keep things simple by not inventing a new format, > > but instead of using 'check.time', use 'check.time.$FORMAT-$PROTOCOL' > > eg 'check.time.qcow2-file' > > Seems more palatable. Lots of files rather than lots of lines in a > single file; searching is now fast (if you can come up with the right > file name, the OS does the searching for you rather than us having to > figure out which (if any) portion of a large file applies). We may > still want a command for easily cleaning everything, and if we do switch > to new file names, we'll still want to clean up the old files. Isn't that command called 'git clean -f' - if we remove 'check.time' from the gitignore file, it'll be deleted by a git clean, without needing to ass the -i flag. I don't think we really need to reinvent a special way to clean just these timing files, particularly since they are not large. Regards, Daniel
On 09/20/2016 09:15 AM, Daniel P. Berrange wrote: >>> I guess we could keep things simple by not inventing a new format, >>> but instead of using 'check.time', use 'check.time.$FORMAT-$PROTOCOL' >>> eg 'check.time.qcow2-file' >> >> Seems more palatable. Lots of files rather than lots of lines in a >> single file; searching is now fast (if you can come up with the right >> file name, the OS does the searching for you rather than us having to >> figure out which (if any) portion of a large file applies). We may >> still want a command for easily cleaning everything, and if we do switch >> to new file names, we'll still want to clean up the old files. > > Isn't that command called 'git clean -f' - if we remove 'check.time' > from the gitignore file, it'll be deleted by a git clean, without > needing to ass the -i flag. I don't think we really need to reinvent > a special way to clean just these timing files, particularly since > they are not large. I'd still rather have the gitignore file ignore anything that we don't want to check into the tree (I'm a big fan of 'git add -a .', and hate it when non-ignored generated files interfere with that; yes I know I can munge .git/info/exclude locally, but it feels cleaner when generated files are ignored at the project level). But yes, relying on git to do the cleanup is fine from my point of view.
On Tue, Sep 20, 2016 at 09:38:33AM -0500, Eric Blake wrote: > On 09/20/2016 09:15 AM, Daniel P. Berrange wrote: > >>> I guess we could keep things simple by not inventing a new format, > >>> but instead of using 'check.time', use 'check.time.$FORMAT-$PROTOCOL' > >>> eg 'check.time.qcow2-file' > >> > >> Seems more palatable. Lots of files rather than lots of lines in a > >> single file; searching is now fast (if you can come up with the right > >> file name, the OS does the searching for you rather than us having to > >> figure out which (if any) portion of a large file applies). We may > >> still want a command for easily cleaning everything, and if we do switch > >> to new file names, we'll still want to clean up the old files. > > > > Isn't that command called 'git clean -f' - if we remove 'check.time' > > from the gitignore file, it'll be deleted by a git clean, without > > needing to ass the -i flag. I don't think we really need to reinvent > > a special way to clean just these timing files, particularly since > > they are not large. > > I'd still rather have the gitignore file ignore anything that we don't > want to check into the tree (I'm a big fan of 'git add -a .', and hate > it when non-ignored generated files interfere with that; yes I know I > can munge .git/info/exclude locally, but it feels cleaner when generated > files are ignored at the project level). > > But yes, relying on git to do the cleanup is fine from my point of view. NB, I'd still suggest ignoring check.time-$FORMAT-BACKEND files since those are current. I was just refering to removal of the legacy check.time file so next git cleanup kills the obsolete file Regards, Daniel
Am 20.09.2016 um 16:39 hat Daniel P. Berrange geschrieben: > On Tue, Sep 20, 2016 at 09:38:33AM -0500, Eric Blake wrote: > > On 09/20/2016 09:15 AM, Daniel P. Berrange wrote: > > >>> I guess we could keep things simple by not inventing a new format, > > >>> but instead of using 'check.time', use 'check.time.$FORMAT-$PROTOCOL' > > >>> eg 'check.time.qcow2-file' > > >> > > >> Seems more palatable. Lots of files rather than lots of lines in a > > >> single file; searching is now fast (if you can come up with the right > > >> file name, the OS does the searching for you rather than us having to > > >> figure out which (if any) portion of a large file applies). We may > > >> still want a command for easily cleaning everything, and if we do switch > > >> to new file names, we'll still want to clean up the old files. > > > > > > Isn't that command called 'git clean -f' - if we remove 'check.time' > > > from the gitignore file, it'll be deleted by a git clean, without > > > needing to ass the -i flag. I don't think we really need to reinvent > > > a special way to clean just these timing files, particularly since > > > they are not large. > > > > I'd still rather have the gitignore file ignore anything that we don't > > want to check into the tree (I'm a big fan of 'git add -a .', and hate > > it when non-ignored generated files interfere with that; yes I know I > > can munge .git/info/exclude locally, but it feels cleaner when generated > > files are ignored at the project level). > > > > But yes, relying on git to do the cleanup is fine from my point of view. > > NB, I'd still suggest ignoring check.time-$FORMAT-BACKEND files since > those are current. I was just refering to removal of the legacy check.time > file so next git cleanup kills the obsolete file Was there a v2 of this series that I missed or was it just forgotten? Kevin
On Mon, Oct 31, 2016 at 03:28:23PM +0100, Kevin Wolf wrote: > Am 20.09.2016 um 16:39 hat Daniel P. Berrange geschrieben: > > On Tue, Sep 20, 2016 at 09:38:33AM -0500, Eric Blake wrote: > > > On 09/20/2016 09:15 AM, Daniel P. Berrange wrote: > > > >>> I guess we could keep things simple by not inventing a new format, > > > >>> but instead of using 'check.time', use 'check.time.$FORMAT-$PROTOCOL' > > > >>> eg 'check.time.qcow2-file' > > > >> > > > >> Seems more palatable. Lots of files rather than lots of lines in a > > > >> single file; searching is now fast (if you can come up with the right > > > >> file name, the OS does the searching for you rather than us having to > > > >> figure out which (if any) portion of a large file applies). We may > > > >> still want a command for easily cleaning everything, and if we do switch > > > >> to new file names, we'll still want to clean up the old files. > > > > > > > > Isn't that command called 'git clean -f' - if we remove 'check.time' > > > > from the gitignore file, it'll be deleted by a git clean, without > > > > needing to ass the -i flag. I don't think we really need to reinvent > > > > a special way to clean just these timing files, particularly since > > > > they are not large. > > > > > > I'd still rather have the gitignore file ignore anything that we don't > > > want to check into the tree (I'm a big fan of 'git add -a .', and hate > > > it when non-ignored generated files interfere with that; yes I know I > > > can munge .git/info/exclude locally, but it feels cleaner when generated > > > files are ignored at the project level). > > > > > > But yes, relying on git to do the cleanup is fine from my point of view. > > > > NB, I'd still suggest ignoring check.time-$FORMAT-BACKEND files since > > those are current. I was just refering to removal of the legacy check.time > > file so next git cleanup kills the obsolete file > > Was there a v2 of this series that I missed or was it just forgotten? I never got around to sending a v2 Regards, Daniel
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index 4cba215..1a9bbb7 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -142,6 +142,11 @@ _timestamp() echo -n " [$now]" } +_timestamp_args() +{ + echo "# ARGS: protocol=$IMGPROTO, format=$IMGFMT" +} + _wrapup() { # for hangcheck ... @@ -158,13 +163,14 @@ _wrapup() if [ -f check.time -a -f $tmp.time ] then cat check.time $tmp.time \ - | $AWK_PROG ' + | grep -v '# ' | $AWK_PROG ' { t[$1] = $2 } END { if (NR > 0) { for (i in t) print i " " t[i] } }' \ | sort -n >$tmp.out + _timestamp_args >> $tmp.out mv $tmp.out check.time fi @@ -223,7 +229,18 @@ echo "preamble" > "${TEST_DIR}"/check.sts # don't leave old full output behind on a clean run rm -f check.full -[ -f check.time ] || touch check.time +# Check if previously recorded times were for the same +# image format/protocol, otherwise discard them +if test -f check.time +then + OLDARGS=`grep ARGS check.time` + NEWARGS=`_timestamp_args` + if test "x$OLDARGS" != "x$NEWARGS" + then + rm -f check.time + fi +fi +touch check.time FULL_IMGFMT_DETAILS=`_full_imgfmt_details` FULL_IMGPROTO_DETAILS=`_full_imgproto_details`
The 'check' program records timings for each test that is run. These timings are only valid, however, for a particular format/protocol combination. So if frequently running 'check' with a variety of different formats or protocols, the times printed can be very misleading. Record the protocol/format in the check.time file and throw it away if it doesn't mach the current run args. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- tests/qemu-iotests/check | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)