[Piglit] [PATCH] programs/run.py add file sync command line option
Atwood, Matthew S
matthew.s.atwood at intel.com
Thu Aug 14 09:55:04 PDT 2014
open_dict is already calling the new file sync. I am hesitant to sync write_dict_key for fear of unnecessary blocking calls. I am also confused as to why that is a public api call, seems like it should only be called by write_dict_item.
-----Original Message-----
From: Dylan Baker [mailto:baker.dylan.c at gmail.com]
Sent: Wednesday, August 13, 2014 3:37 PM
To: Atwood, Matthew S
Cc: piglit at lists.freedesktop.org; Chad Versace
Subject: Re: [Piglit] [PATCH] programs/run.py add file sync command line option
Chad might have another opinion on the matter, but I think at a mininum the public methods should be synced, so open_dict, and write_dict_key should probably also be synched
On Wednesday, August 13, 2014 10:34:59 PM Atwood, Matthew S wrote:
> I specifically avoided calling it in helper functions to ensure that we weren't unnecessarily syncing, we don't particularly care about a half done test case being written, its either all or nothing in terms or whether or not we care I think.
>
> -----Original Message-----
> From: Dylan Baker [mailto:baker.dylan.c at gmail.com]
> Sent: Wednesday, August 13, 2014 3:24 PM
> To: piglit at lists.freedesktop.org
> Cc: Atwood, Matthew S
> Subject: Re: [Piglit] [PATCH] programs/run.py add file sync command
> line option
>
> On Monday, January 02, 2012 05:09:08 PM Matthew Atwood wrote:
> > From: Matt Atwood <matthew.s.atwood at intel.com>
> >
> > Currently while running igt tests a kernel panic causes the results
> > json file to lose all data. This patch adds a command line option
> > (-s,
> > --sync) that syncs the file descriptor to disk after every test
> > allowing data to persist through hard crashes.
> > ---
> > framework/core.py | 3 ++-
> > framework/programs/run.py | 14 ++++++++++----
> > framework/results.py | 12 +++++++++++-
> > 3 files changed, 23 insertions(+), 6 deletions(-)
> >
> > diff --git a/framework/core.py b/framework/core.py index
> > d3922a9..8c4ff57 100644
> > --- a/framework/core.py
> > +++ b/framework/core.py
> > @@ -100,7 +100,7 @@ class Options(object):
> > """
> > def __init__(self, concurrent=True, execute=True, include_filter=None,
> > exclude_filter=None, valgrind=False, dmesg=False,
> > - verbose=False):
> > + verbose=False, sync=False):
> > self.concurrent = concurrent
> > self.execute = execute
> > self.filter = [re.compile(x) for x in include_filter or []]
> > @@ -109,6 +109,7 @@ class Options(object):
> > self.valgrind = valgrind
> > self.dmesg = dmesg
> > self.verbose = verbose
> > + self.sync = sync
> > # env is used to set some base environment variables that are not going
> > # to change across runs, without sending them to os.environ which is
> > # fickle as easy to break
> > diff --git a/framework/programs/run.py b/framework/programs/run.py
> > index eb67d7f..468bff4 100644
> > --- a/framework/programs/run.py
> > +++ b/framework/programs/run.py
> > @@ -97,6 +97,9 @@ def run(input_):
> > type=path.realpath,
> > metavar="<Results Path>",
> > help="Path to results folder")
> > + parser.add_argument("-s", "--sync",
> > + action="store_true",
> > + help="Sync results to disk after every
> > + test")
> > args = parser.parse_args(input_)
> >
> > # Disable Windows error message boxes for this and all child processes.
> > @@ -132,7 +135,8 @@ def run(input_):
> > execute=args.execute,
> > valgrind=args.valgrind,
> > dmesg=args.dmesg,
> > - verbose=args.verbose)
> > + verbose=args.verbose,
> > + sync=args.sync)
> >
> > # Set the platform to pass to waffle
> > if args.platform:
> > @@ -154,7 +158,7 @@ def run(input_):
> > # Begin json.
> > result_filepath = path.join(args.results_path, 'results.json')
> > result_file = open(result_filepath, 'w')
> > - json_writer = framework.results.JSONWriter(result_file)
> > + json_writer = framework.results.JSONWriter(result_file,
> > + opts.sync)
> >
> > # Create a dictionary to pass to initialize json, it needs the contents of
> > # the env dictionary and profile and platform information @@
> > -211,7 +215,8 @@ def resume(input_):
> > execute=results.options['execute'],
> > valgrind=results.options['valgrind'],
> > dmesg=results.options['dmesg'],
> > - verbose=results.options['verbose'])
> > + verbose=results.options['verbose'],
> > + sync=results.options['sync'])
> >
> > core.get_config(args.config_file)
> >
> > @@ -219,7 +224,8 @@ def resume(input_):
> > opts.env['PIGLIT_PLATFORM'] = results.options['platform']
> >
> > results_path = path.join(args.results_path, 'results.json')
> > - json_writer = framework.results.JSONWriter(open(results_path, 'w+'))
> > + json_writer = framework.results.JSONWriter(open(results_path, 'w+'),
> > + opts.sync)
> > json_writer.initialize_json(results.options, results.name,
> > core.collect_system_info())
> >
> > diff --git a/framework/results.py b/framework/results.py index
> > 4b5fb30..d39f63b 100644
> > --- a/framework/results.py
> > +++ b/framework/results.py
> > @@ -102,8 +102,9 @@ class JSONWriter(object):
> >
> > INDENT = 4
> >
> > - def __init__(self, f):
> > + def __init__(self, f, fsync):
> > self.file = f
> > + self.fsync = fsync
> > self.__indent_level = 0
> > self.__inhibit_next_indent = False
> > self.__encoder = json.JSONEncoder(indent=self.INDENT,
> > @@ -175,6 +176,11 @@ class JSONWriter(object):
> > self.file.close()
> >
> > @synchronized_self
> > + def __file_sync(self):
> > + if self.fsync:
> > + os.fsync(self.file)
> > +
> > + @synchronized_self
> > def __write_indent(self):
> > if self.__inhibit_next_indent:
> > self.__inhibit_next_indent = False @@ -201,6 +207,7 @@
> > class JSONWriter(object):
> > self.__indent_level += 1
> > self.__is_collection_empty.append(True)
> > self._open_containers.append('dict')
> > + self.__file_sync()
> >
> > @synchronized_self
> > def close_dict(self):
> > @@ -212,6 +219,7 @@ class JSONWriter(object):
> > self.file.write('}')
> > assert self._open_containers[-1] == 'dict'
> > self._open_containers.pop()
> > + self.__file_sync()
> >
> > @synchronized_self
> > def write_dict_item(self, key, value):
> > @@ -221,6 +229,8 @@ class JSONWriter(object):
> > # Write value.
> > self.__write(value)
> >
> > + self.__file_sync()
> > +
> > @synchronized_self
> > def write_dict_key(self, key):
> > # Write comma if this is not the initial item in the dict.
>
> I don't think that you've used your new method everywhere you want to use it, I think that you need to call it at the end of each method except __init__.
>
> I think a decorator would be an elegant way to solve this, but it would be a rather complex decorator (or, actually probably a pair of decorators), and it would probably mean working on the synchronized_self decorator as well, since I strongly suspect there would be a bad interaction there.
>
> > --
> > 1.8.3.2
> >
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit
More information about the Piglit
mailing list