Benchmark tool

[nwinn-student]

The parallelbench tool in #33 is not ideal. Parallel benchmark results are wildly inaccurate.

When benchmarking, what do we need to be able to do?

• Specify the path to run
• Skip certain files that we don't want to run
• Store the results for future comparison or viewing via other tools
• Process generalized benchmark data obtained from the ran files into the stored result.

Usage: benchmark [options] <path>

Available options:
  --skip<=path[,path]>: Excludes the specified paths, relative to the provided path, from running
  --output<=file>: Which file to store the results in.

The JSON format the output uses is as follows:

type BenchmarkData = {
  sample_size: number,

  median: number,
  iqr: number,
  mad: number, -- median absolute deviation ( median(|x_i - median|) )
  mild_outliers: number, -- q1 - 1.5 * iqr and q3 + 1.5 * iqr

  min: number,
  average: number,
  max: number,
  stdev: number,
}

type Data = {
  time: BenchmarkData,
  space: BenchmarkData,
  metadata: { [string]: any }?
}

type Format = {
  [string]: Data | Format
}

Assumptions:

• A progress bar will be used, because I like it, but printed messages in the benchmark will be overwritten.
• Enforced: Each ran luau file returns a value of type { time: { number }, space: { number }, metadata: { [string]: any }? }

[nwinn-student]

The tool would need to handle potentially 100s of benchmarks of various depth, it is very difficult to know which approach would be the best. We could categorize, and read metadata.category.

We are making a tool for BufferSerializer. We are not making a tool for everything. Narrow the scope. I will use the defined JSON format since it is already quite broad. BUT I can make my own comparison tools that aren't public and thus don't need to be subject to scrutiny.

I have a better idea. What if we instead look for tools that can create benchmark comparison graphs, then see what format they use and conform to the one that has the most support. That will save alot of time and I am not good at making visualizations fast yet.

We can make rough visualizations using tables and Markdown. Other tools save time, but they won't be able to meet my needs completely.

[nwinn-student]

No. The path name is used by default as the tag.

Wait. If the path is a directory, a tag being set would destroy the benchmarks, leaving one.

[nwinn-student]

No. The path name is used by default as the tag.

Since it isn't required, and it can at most be surface level, just remove it.

[nwinn-student]

No. The path name is used by default as the tag.

Since it isn't required, and it can at most be surface level, just remove it.

Actually, it is still useful for versioning. But if you need to version, it would be better to place the necessary files into a directory with the version name.

I.e:

/bufferserializer/vX.Y.Z instead of /bufferserializer.

[nwinn-student]

Actually, it is still useful for versioning.

Well, it would be better to have a separate tool that does that, but it would be too bs specific.

[nwinn-student]

How do we append results to the given output?

Do we read it in, then modify the data, then store?

Reading the json file in sounds decent enough, should the output file exist already.

What if we want to overwrite the output file?

If they want to overwrite, they can just specify a different file. Also, it will overwrite some if not all of the data if benchmarking the same directories.

[nwinn-student]

Should be simple. string.find(subject: string, pattern: string, position: number?, plain: boolean?) where we just set plain to be true, since in parallelbench we just use the subject and pattern parameters.

[nwinn-student]

Is skip even necessary? And how is it defined?

Does it skip all directories / files that are in the list or does it only skip the topmost or what?

What if a user wants to skip a file called "helloworld" but there are other files whose names contain it deeper in the directory tree?

Skip worked fine and was easy to understand when handling a directory containing only files. But if the directory contains directories it becomes much more nuanced.

[nwinn-student]

Is skip even necessary?

Skip is required, since we often need to exclude the execution of some benchmarks. Sometimes it is to speed up the process to check if additions caused regressions and we know only certain benchmarks could have been impacted.

And how is it defined?

Skip is now defined as the ability to exclude a directory or file or series of directories or files/directories that whose name starts with an element in the skip table.

Assume we have a directory tree:

foo
  bar
    moo.lua
    meow.luau
  baz
    bark.luau
    leaf.luau
    bar
      mushroom.lua
      fungi.luau
      

If we say:
lune run tools/benchmark.luau --skip=bar/m foo

It will not run foo/bar. However, it depends on whether the results will be defined as { foo = ... } or { bar = ..., baz = ... }. If the first, then it will run everything, you would need to skip foo/bar/m to achieve the desired result. If the second, then it will not run any files in foo/bar.

Does it skip all directories / files that are in the list or does it only skip the topmost or what?

It only skips the files/directories from the root as specified.

What if a user wants to skip a file called "helloworld" but there are other files whose names contain it deeper in the directory tree?

It will only skip the file when the path has been provided.

[nwinn-student]

It will not run foo/bar. However, it depends on whether the results will be defined as { foo = ... } or { bar = ..., baz = ... }. If the first, then it will run everything, you would need to skip foo/bar/m to achieve the desired result. If the second, then it will not run any files in foo/bar.

I prefer the first, since it allows for individual files to not wipe the entire file. So it'll add depth, but ensure users can run individual benchmarks if needed.

[nwinn-student]

But we can just initially do: require("../" .. path), since we are in the tools dir and need to start from the current working directory which we wrongly assume will always be the repo directory.

Forcing the user to specify the path relative to the benchmark tool would work. However, it takes away the benefit of being able to locate the file/directory containing the benchmarks easily.

I say we keep our wrong assumption for ease, and acknowledge that the tool cannot be easily used until later iterations that can somehow know the path of the benchmark tool relative to the cwd.

[nwinn-student]

somehow know the path of the benchmark tool relative to the cwd.

Preferably we find this out. There may be a way using a bash command, but not all platforms have the same tools or syntax.

echo $0??? Will need to check it out.

[nwinn-student]

somehow know the path of the benchmark tool relative to the cwd.

Preferably we find this out. There may be a way using a bash command, but not all platforms have the same tools or syntax.

echo $0??? Will need to check it out.

I forgot about debug.info. I think the source may help. Hopefully Lune does relative source names from cwd.

[nwinn-student]

I forgot about debug.info. I think the source may help. Hopefully Lune does relative source names from cwd.

It gives the absolute path of the file, but that is fine. Just have to find out how to make it relative.

[nwinn-student]

I forgot about debug.info. I think the source may help. Hopefully Lune does relative source names from cwd.

It gives the absolute path of the file, but that is fine. Just have to find out how to make it relative.

Done. Took the lazy approach where we go to root then back.

[nwinn-student]

Do nothing. It may be a mistake, but some users may print stuff and pipe it to a file instead.

[nwinn-student]

I chose to give up on it. process.create will not allow for the stdout to be updated in-real time, which is what I needed. I was planning on passing the subprocess input regarding which files are being run. The subprocess would then modify the output displayed. Then when we say the files are done, we remove the printed material then kill the subprocess. Then print, if needed, the results.

I tried process.exec, but it has issues of concurrent running, so it cannot be run in parallel with the benchmarks.

[nwinn-student]

I did not consider stdio.write, but I presume it is identical to print. And I also presume it just writes to the stdout for the subprocess, I have yet to test, and likely won't.

[nwinn-student]

Do we need the view option? It can become quite hairy the more benchmarks are run, even for the summarized versions.

foo
 bar | 0e-0 s | 0e-0 kB
 baz | 0e-0 s | 0e-0 kB
goo
 var | 0e-0 s | 0e-0 kB
 vaz | 0e-0 s | 0e-0 kB
doo
 car
  xoo
   zar | 0e-0 s | 0e-0 kB
   zaz | 0e-0 s | 0e-0 kB
 caz
  zoo
   mar | 0e-0 s | 0e-0 kB
   maz | 0e-0 s | 0e-0 kB

It does not look too bad, but I can see it being harder to read. I do not think that is a valid reason to dismiss the feature though.

[nwinn-student]

It does not look too bad, but I can see it being harder to read. I do not think that is a valid reason to dismiss the feature though.

Maybe a better reason would be that other means of visualizing the information are more optimal for more extensive benchmark suites. Textual is great under the assumption of 20 or less benchmarks to compare, and you may want to see the results from prior runs.

[nwinn-student]

We could force that an output path is set to avoid the situation where it isn't and --view isn't set.

I feel like we should instead have a default path. benchmark-results.json in the cwd path. So if nothing is specified, it does that.

We will need to be clear in --help about that.

[nwinn-student]

We could force that an output path is set to avoid the situation where it isn't and --view isn't set.

I feel like we should instead have a default path. benchmark-results.json in the cwd path. So if nothing is specified, it does that.

We will need to be clear in --help about that.

bench/results.json is also good. But I will choose benchmark-results.json to avoid creating directories.

[nwinn-student]

It does not look too bad, but I can see it being harder to read. I do not think that is a valid reason to dismiss the feature though.

Maybe a better reason would be that other means of visualizing the information are more optimal for more extensive benchmark suites. Textual is great under the assumption of 20 or less benchmarks to compare, and you may want to see the results from prior runs.

So basically, remove --view in favor of a separate tool to reduce the scope of the benchmark tool. Alright, sounds fine.

[nwinn-student]

We could require the path, which would mean that the path needs to be relative to the runner and that it needs to call the other benchmarks. We could then print the results at the end, and have the users get it into a file. Although, there wouldn't be much use since the user would have to spend alot of time setting it up properly.

The current approach fits quite well with automating the files to run. Although this new approach can be automated as well. There are issues with the current, such as requiring the Lune runtime, since embedders may want to measure their custom userdata. But the purpose is to support Luau code, not embedded, although Lune code specifically can be tested too.

[nwinn-student]

We could require the path, which would mean that the path needs to be relative to the runner and that it needs to call the other benchmarks. We could then print the results at the end, and have the users get it into a file. Although, there wouldn't be much use since the user would have to spend alot of time setting it up properly.

We would also be unable to skip, so everything would be user-defined. Which is fine, but has very little use.

The tool code would basically just be:

local args = {...}
local data = require(args[1])

prettyprint(data)

[nwinn-student]

If you have hundreds of benchmarks and you have already run 90, it is easier to include than exclude.

[nwinn-student]

A plain check first then a pattern matching check

Order doesn't matter since we will just say X or Y.

[nwinn-student]

While comparing for regressions in versions is one of the reasons, it is not the main one. The main purpose, now, is to identify which dataset categories BufferSerializer is good at and direct future patches and updates towards improving what it is good at and loosely improving everything else.

It is clear that BufferSerializer is already tailored subconsciously towards redundant datasets, however how do nested datasets fair. Knowing the pros and cons and having evidence to prove them is important.

Instead of saying that between version N and version M serialization time was reduced by 10%, I can say that certain categories of datasets will improve by B% and others by V%. Maybe some regress. The current approach is inflexible in this regard where I can merely say the best or worst case I observe.

[nwinn-student]

So, "give me the data and I will process it, else I expect for you to have processed it"?

[nwinn-student]

Also I could change it to expect { [string]: {...} }, since the implementation could be changed to that, but I feel like it is too harsh.

I have now realized that the solution, instead of modifying the generated code, is to just do nothing in that department. I can delete the files whenever I need to perform a commit and regenerate after. I can even easily build it into the cli I run before every commit in this repo.

[nwinn-student]

Each ran luau file returns a value of type { time: { number }, space: { number }, metadata: { [string]: any}? }
We should back off. We can say that we expect the type, but we don't enforce it. In the current implementation we sort of enforce it, which makes it so that those who know the return format already need to do extra work.

Should, but won't. We are working under assumptions because that helps simplify our life. I would rather be certain that benchmark results from various serialization formats can coincide. If I allow for anything else then I cannot guarantee the output format is followed, since a user could accidentally return metedata instead of metadata. I may even double down to protect the users and error when a flaw is spotted.

I get the sentiment. I may even need to account for the possibility the format is followed for time/space so the assumption can be voided. I cannot however allow for users to try to replicate the actual internal data format, it is a waste of their time. It may benefit storage and analysis times, but such aspects are trivial since the scripts are generated and can be regenerated on-demand. Not all usage cases will involve generating scripts, but those that do will have users that try to be smart and reduce file count, which is not worth the hassle.

[nwinn-student]

Not much needs to change other than removing the restrictions and expecting I know what I am doing, as mentioned in some other comments.

I will be keeping, possibly enforcing, the restrictions. I know what I am doing, but that justification does not apply to others. I cannot expect others using the tool to understand.

[nwinn-student]

I was looking at other benchmarking tools surrounding luau and I like how bench has some way to look at stability and outliers. I think that we need to account for both and store values that can be used in tools to either generate the values or outright add them to the format.

I saw mad, cv, and mild outliers. cv is the coefficient of variation, in the repo they store it as a percent and is calculated using stdev and avg. Mild outliers are calculated using the iqr and all of the data. mad is the median absolute deviation and is calculated using the median and all of the data. From this, I think we should add mad and mild outlier. There were also severe outliers, but I do not think that value is as useful.

I also saw a confidence interval used interally, but it just requires the count, which is used to calculate the standard error and t-critical, then the margin for the interval.

I propose we add mad: number -- median absolute deviation ( median(|x_i - median|) ), mild_outliers: number -- q1 - 1.5 * iqr and q3 + 1.5 * iqr, and sample_size: number -- number of kept runs to BenchData.

[nwinn-student]

These should be directed towards luau-bench, and luau-bench should just be internally in tools so users can use it, then we can harden the accepted format to be the specified one w/o a { number } option. It makes life overall easier.

[nwinn-student]

Remove hist. The information it holds can be inferred by stdev and mad. Knowing how many are in some set range is useful for visualizations, but storing the entire dataset would allow for more precise visualizations.

[nwinn-student]

Alternatives would be to keep it as { [string]: number } where the user has to refer to this to make sure their regex or language-specific pattern matching works properly. Or add range and upper/lower outlier properties to allow users to recalculate the values.

Both of which cause unnecessary work for the user.