[igt-dev] [PATCH i-g-t 6/8] benchmarks/gem_wsim: allow comments in workload description files
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Mon Sep 25 09:03:13 UTC 2023
On 21/09/2023 17:20, Bernatowicz, Marcin wrote:
>
>
> On 9/21/2023 5:22 PM, Tvrtko Ursulin wrote:
>>
>> On 21/09/2023 16:05, Bernatowicz, Marcin wrote:
>>> Hi,
>>>
>>> On 9/20/2023 6:13 PM, Tvrtko Ursulin wrote:
>>>>
>>>> On 06/09/2023 16:51, Marcin Bernatowicz wrote:
>>>>> Lines starting with '#' are skipped.
>>>>> If command line step separator (',') is encountered after '#'
>>>>> it is replaced with ';' to not break parsing.
>>>>>
>>>>> Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz at linux.intel.com>
>>>>> ---
>>>>> benchmarks/gem_wsim.c | 41
>>>>> ++++++++++++++++++++++++++++++++---------
>>>>> benchmarks/wsim/README | 2 ++
>>>>> 2 files changed, 34 insertions(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/benchmarks/gem_wsim.c b/benchmarks/gem_wsim.c
>>>>> index 0c1b58727..ec9fdc2d0 100644
>>>>> --- a/benchmarks/gem_wsim.c
>>>>> +++ b/benchmarks/gem_wsim.c
>>>>> @@ -43,6 +43,7 @@
>>>>> #include <limits.h>
>>>>> #include <pthread.h>
>>>>> #include <math.h>
>>>>> +#include <ctype.h>
>>>>> #include "drm.h"
>>>>> #include "drmtest.h"
>>>>> @@ -94,6 +95,7 @@ enum w_type {
>>>>> TERMINATE,
>>>>> SSEU,
>>>>> WORKINGSET,
>>>>> + SKIP,
>>>>> };
>>>>> struct dep_entry {
>>>>> @@ -930,6 +932,12 @@ parse_workload(struct w_arg *arg, unsigned int
>>>>> flags, double scale_dur,
>>>>> if (field) {
>>>>> fstart = NULL;
>>>>> + /* line starting with # is a comment */
>>>>> + if (field[0] == '#') {
>>>>> + step.type = SKIP;
>>>>> + goto add_step;
>>>>> + }
>>>>
>>>> Do they need to be recorded as steps and couldn't be simply silently
>>>> skipped over while parsing?
>>>
>>> Looks indeed a bool skip may be enough. It's a dummy step not stored
>>> in workload steps.
>>
>> Cool, that would be the best.
>>
>>>>
>>>> How does relative step referencing works when comments are present?
>>>> (Batch implicit dependencies and 'a' and 's' commands.)
>>>
>>> Comments may be added to existing workloads without a problem as the
>>> SKIPs are not incrementing nr_step (does not create a record).
>>
>> Oookay, guess I was confused with the goto add_step. :)
>>
>>> Maybe a small confusion is that step.idx does not correspond to line
>>> number, so in case of failed parse we get a step number without a
>>> line :/
>>
>> Hm? If parse fails the whole program fails so I didn't get this.
>
> Sorry, I created a confusion. A Failure message does not point the line
> number but the step number (0-based) - that didn't change.
> Looking at workload definition (broken for example) in text editor
> having line number could easy a bit:
>
> 1 # Workload simulating transcoding session
> 2 # ex. gem_wsim -w benchmarks/wsim/media_load_balance_fhd26u7.wsim -c
> 36 -r 600
> 3 # will run 36 parallel transcoding session streams for 600 frames each
> 4 M.3.VCS
> 5 B.3
> 6 1.VCS1.1200-1800.0.0
> 7 1.VCS1.1900-2100.0.0
> 8 2.RCS.1500-2000.-1.0
> 9 3.VCS.1400-1800.-1.1
> 10 1.VCS1.1900-2100.-1.0
> 11 2.RCS.1500-2000.-1.0
> 12 3.VCS.1400-1800.-1.1
> 13 1.VCS1.1900-2100.-1.0
> 14 2.RCS.200-400.-1.0
> 15 2.RCS.1500-2000.0.0
> 16 3.VCS.1400-1800.-1.1
> 17 1.VCS1.1900-2100.-1.0
> 18 2.RCS.1500-2000.-1.0
> 19 3.VCS.1400-1800.-1.1
> 20 1.VCS1.1900-2100.-1.0
> 21 2.RCS.200-400.-1.0
> 22 2.RCS.1500-2000.0.0
> 23 3.VCS.1400-k1800.-1.1
> 24 1.VCS1.1900-2100.-1.0
> 25 2.RCS.1500-2000.-1.0
> 26 3.VCS.1400-1800.-1.1
> 27 1.VCS1.1900-2100.-1.0
> 28 2.RCS.1500-2000.-1.0
> 29 2.RCS.1500-2000.0.0
> 30 3.VCS.1400-1800.-1.1
>
> Current parse failure message will tell:
>
> Invalid duration at step 19!
> Failed to parse workload 0!
>
> Which was on line 20 (step no + 1) and now is on 23 (+ no of comment
> lines).
> But there are more important matters :)
Ah right.. yes it would be handy to have better parsing error reporting.
I think it is easy to do. Just store line numbers in struct w_step.
Heck, we could strdup the whole line while parsing and go super fancy by
showing it verbatim on error, like:
Invalid duration at step 19, line 23!
>>> '23 3.VCS.1400-k1800.-1.1'
Failed to parse workload 0!
Regards,
Tvrtko
>
> Regards,
> Marcin
>
>>
>> Regards,
>>
>> Tvrtko
>>
>>>
>>> Regards,
>>> Marcin
>>>>
>>>> Regards,
>>>>
>>>> Tvrtko
>>>>
>>>>> +
>>>>> if (!strcmp(field, "d")) {
>>>>> int_field(DELAY, delay, tmp <= 0,
>>>>> "Invalid delay at step %u!\n");
>>>>> @@ -1194,7 +1202,8 @@ parse_workload(struct w_arg *arg, unsigned
>>>>> int flags, double scale_dur,
>>>>> if (field) {
>>>>> fstart = NULL;
>>>>> - check_arg(strlen(field) != 1 ||
>>>>> + check_arg(!strlen(field) ||
>>>>> + (strlen(field) > 1 && !isspace(field[1]) &&
>>>>> field[1] != '#') ||
>>>>> (field[0] != '0' && field[0] != '1'),
>>>>> "Invalid wait boolean at step %u!\n",
>>>>> nr_steps);
>>>>> @@ -1208,18 +1217,23 @@ parse_workload(struct w_arg *arg, unsigned
>>>>> int flags, double scale_dur,
>>>>> step.type = BATCH;
>>>>> add_step:
>>>>> - if (step.type == DELAY)
>>>>> - step.delay = __duration(step.delay, scale_time);
>>>>> + if (step.type == SKIP) {
>>>>> + if (verbose > 3)
>>>>> + printf("skipped STEP: %s\n", _token);
>>>>> + } else {
>>>>> + if (step.type == DELAY)
>>>>> + step.delay = __duration(step.delay, scale_time);
>>>>> - step.idx = nr_steps++;
>>>>> - step.request = -1;
>>>>> - steps = realloc(steps, sizeof(step) * nr_steps);
>>>>> - igt_assert(steps);
>>>>> + step.idx = nr_steps++;
>>>>> + step.request = -1;
>>>>> + steps = realloc(steps, sizeof(step) * nr_steps);
>>>>> + igt_assert(steps);
>>>>> - memcpy(&steps[nr_steps - 1], &step, sizeof(step));
>>>>> + memcpy(&steps[nr_steps - 1], &step, sizeof(step));
>>>>> + }
>>>>> free(token);
>>>>> - }
>>>>> + } // while ((_token = strtok_r(tstart, ",", &tctx))) {
>>>>> if (app_w) {
>>>>> steps = realloc(steps, sizeof(step) *
>>>>> @@ -2304,6 +2318,8 @@ static void *run_workload(void *data)
>>>>> enum intel_engine_id engine = w->engine;
>>>>> int do_sleep = 0;
>>>>> + igt_assert(w->type != SKIP);
>>>>> +
>>>>> if (w->type == DELAY) {
>>>>> do_sleep = w->delay;
>>>>> } else if (w->type == PERIOD) {
>>>>> @@ -2543,6 +2559,13 @@ static char *load_workload_descriptor(char
>>>>> *filename)
>>>>> close(infd);
>>>>> for (i = 0; i < len; i++) {
>>>>> + /* '#' starts comment till end of line */
>>>>> + if (buf[i] == '#')
>>>>> + /* replace ',' in comments to not break parsing */
>>>>> + while (++i < len && buf[i] != '\n')
>>>>> + if (buf[i] == ',')
>>>>> + buf[i] = ';';
>>>>> +
>>>>> if (buf[i] == '\n')
>>>>> buf[i] = ',';
>>>>> }
>>>>> diff --git a/benchmarks/wsim/README b/benchmarks/wsim/README
>>>>> index 8c71f2fe6..e4fd61645 100644
>>>>> --- a/benchmarks/wsim/README
>>>>> +++ b/benchmarks/wsim/README
>>>>> @@ -1,6 +1,8 @@
>>>>> Workload descriptor format
>>>>> ==========================
>>>>> +Lines starting with '#' are treated as comments (do not create
>>>>> work step).
>>>>> +
>>>>> ctx.engine.duration_us.dependency.wait,...
>>>>> <uint>.<str>.<uint>[-<uint>]|*.<int <= 0>[/<int <=
>>>>> 0>][...].<0|1>,...
>>>>> B.<uint>
More information about the igt-dev
mailing list