VCompressor Benchmark

[Sefank]

As #249 mentioned, VCompressor is designed for a decent compress ratio. Here provides a benchmark framework to easily calculate performance improvement space-wise and time-wise on specific test cases.

Note: Here, I chose to use LZMA, famous for its high compress ratio on average files, as a baseline in terms of space-wise benchmark.

---

Main Features:

• Space-wise Benchmark
• Time-wise Benchmark
• Display compress ratio in the vcompressor benchmark
• Support multiple other compressors for comparison

Other Important Change Log:

• FIX: custom repeat(previously loop_time) in _benchmark is now implemented correctly
• STYLE: All related functions and types are now placed in a single class, without affecting global namespace

---

Output Example of this Benchmark Test:

2022-09-05 23:01:24,684 INFO: tests.test_vcompressor.TestVCompressorPerformance.test_benchmark_basic start
2022-09-05 23:01:26,302 INFO: 1. On file "vdb_basic.json":
2022-09-05 23:01:26,302 INFO:     [Space]
2022-09-05 23:01:26,302 INFO:       Uncompressed:       2.95KB
2022-09-05 23:01:26,302 INFO:       VCompressor:      531.00B(1.000) [CR: 17.57%]
2022-09-05 23:01:26,302 INFO:       LZMA:             504.00B(0.949) [CR: 16.68%]
2022-09-05 23:01:26,303 INFO:     [Time]
2022-09-05 23:01:26,303 INFO:       VCompressor:        0.405s(1.000)
2022-09-05 23:01:26,303 INFO:       LZMA:               0.004s(0.010)
2022-09-05 23:01:27,757 INFO: 2. On file "multithread.json":
2022-09-05 23:01:27,757 INFO:     [Space]
2022-09-05 23:01:27,757 INFO:       Uncompressed:     105.22KB
2022-09-05 23:01:27,757 INFO:       VCompressor:        9.05KB(1.000) [CR:  8.60%]
2022-09-05 23:01:27,757 INFO:       LZMA:              16.66KB(1.840) [CR: 15.83%]
2022-09-05 23:01:27,757 INFO:     [Time]
2022-09-05 23:01:27,757 INFO:       VCompressor:        0.330s(1.000)
2022-09-05 23:01:27,757 INFO:       LZMA:               0.023s(0.071)
2022-09-05 23:01:27,757 INFO: tests.test_vcompressor.TestVCompressorPerformance.test_benchmark_basic finish

[gaogaotiantian]

It would be nice to show the compress ratio for each method.

[Sefank]

It would be nice to show the compress ratio for each method.

@gaogaotiantian It seems that compress ratio is defined by compressed_filesize / uncompressed_filesize academically, and anothor common definition is 100% - (compressed_filesize / uncompressed_filesize). idk which one would be appropriate here?

[gaogaotiantian]

Could you rebase your feature branch on origin/trace-log-compressor to enable to CI?

[Sefank]

Could you rebase your feature branch on origin/trace-log-compressor to enable to CI?

I've merge this feature branch on my origin/trace-log-compressor branch and it seems nothing happened?

[gaogaotiantian]

You need to pull the updates from upstream.

# add upstream
git remote add upstream https://github.com/gaogaotiantian/viztracer
# checkout your own branch
git checkout trace-log-compressor
# get the updates from my branch
git pull upstream trace-log-compressor
# checkout your feature branch
git checkout trace-log-compressor-benchmark
# rebase on your updated trace-log-compressor
git rebase trace-log-compressor
# You need -f to force update your feature branch as it is rebased
git push -f origin trace-log-compressor-benchmark

[Sefank]

You need to pull the updates from upstream.

# add upstream
git remote add upstream https://github.com/gaogaotiantian/viztracer
# checkout your own branch
git checkout trace-log-compressor
# get the updates from my branch
git pull upstream trace-log-compressor
# checkout your feature branch
git checkout trace-log-compressor-benchmark
# rebase on your updated trace-log-compressor
git rebase trace-log-compressor
# You need -f to force update your feature branch as it is rebased
git push -f origin trace-log-compressor-benchmark

OK done. Many thanks!

[Sefank]

aha it seems we have a prerequisite pr #273 that need to be reviewed first :D

[gaogaotiantian]

#273 is merged in and could you rebase on that again? Thanks!

[Sefank]

#273 is merged in and could you rebase on that again? Thanks!

Done!

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (trace-log-compressor@fc3bd6a). Click here to learn what that means.
The diff coverage is n/a.

@@                   Coverage Diff                   @@
##             trace-log-compressor     #268   +/-   ##
=======================================================
  Coverage                        ?   98.68%           
=======================================================
  Files                           ?       21           
  Lines                           ?     2205           
  Branches                        ?        0           
=======================================================
  Hits                            ?     2176           
  Misses                          ?       29           
  Partials                        ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[Sefank]

@gaogaotiantian The reason why I determined to put _benchmark decorator outside the class scope is that idk how to make it compatible with the decoration mechanism and class method binding mechanism.

• If I move the _benchmark decorator into the class definition (of course adding self parameter first), it would just throw an error:

  TypeError: _benchmark() missing 1 required positional argument: 'benchmark_process'

  I believed that it is because decorator functions should always wrap the first parameter, which would conflict with the requirement of a method function.

• And then, I tried to add @staticmethod for _benchmark to avoid the self being the first parameter, and it would still throw an error:

  TypeError: 'staticmethod' object is not callable

  idk what's happened.

---

When the _benchmark decorator is outside the class scope, putting BenchmarkResult outside the class scope is for reference simplicity, as the class name TestVCompressorPerformance is too long. If I put it inside the class scope, I have to use it like TestVCompressorPerformance.BenchmarkResult.

[gaogaotiantian]

You only need to move the whole _benchmark function into the class without adding the self argument.

The other thing worth mentioning is that your loop_time argument of the decorator can not be used. You need some extra code to make that work.

[Sefank]

You only need to move the whole _benchmark function into the class without adding the self argument.
The other thing worth mentioning is that your loop_time argument of the decorator can not be used. You need some extra code to make that work.

I've tried to write a minimal demo on the problem:

class Demo:
    def deco(*args, **kargs):
        print(args)
        print(kargs)
        def _wrapper(*args, **kargs):
            pass
        return _wrapper

    @deco
    def func(self):
        pass

demo = Demo()
demo.func()

And this code works as expected and the output is as follows.

(<function Demo.func at 0x7f54138b48c8>,)
{}

"as expected " just means the first positional argument is the decorated function. However, I'm stil confused about why it would work that the first positional argument of deco method is not a "self" object, or not a Demo instance here.

If I add a randomly chosen parameter for the deco like this:

# ...
    @deco(p=5)
    def func(self):
        pass
# ...

It will throw an errror:

()
{'p': 5}
Traceback (most recent call last):
  File "demo.py", line 14, in <module>
    demo.func()
TypeError: 'NoneType' object is not callable

According to the output, the argument that should be the decorated function is magically missing. Only parameter(s) for the decorator is/are left. And it seems that the return value of the decorator changed to a None instead of the wrapper function.

If now I change the return value of the wrapper function like this:

# ...
    def deco(*args, **kargs):
        print(args)
        print(kargs)
        def _wrapper(*args, **kargs):
            return 123
        return _wrapper
# ...

Error info:

TypeError: 'int' object is not callable

It seems that the return value of the decorator changes to the return value of the wrapper function.

I'm more confused right now. Is there any part that I miss? Or is it a worthy point that a new video can be made for? 😄

[gaogaotiantian]

This is a two-piece question. I'll make a video about the decorator in class part because that's actually a pretty common requirements. However, the answer could still be found(with some thoughts) in

https://www.bilibili.com/video/BV1mB4y1m7FT/

It seems that the return value of the decorator changes to the return value of the wrapper function.

I'd recommend https://www.bilibili.com/video/BV19U4y1d79C/ for its answer. It's not about the class anymore. It's about how to make a decorator support usage with and without arguments. Like I said in my previous comment, your loop_time will never work, even in the globally defined decorator.

[Sefank]

This is a two-piece question. I'll make a video about the decorator in class part because that's actually a pretty common requirements. However, the answer could still be found(with some thoughts) in

https://www.bilibili.com/video/BV1mB4y1m7FT/

It seems that the return value of the decorator changes to the return value of the wrapper function.

I'd recommend https://www.bilibili.com/video/BV19U4y1d79C/ for its answer. It's not about the class anymore. It's about how to make a decorator support usage with and without arguments. Like I said in my previous comment, your loop_time will never work, even in the globally defined decorator.

OK. I'll check it out. Thanks again!

[Sefank]

I've put up an output example of this benchmark test in the top post.

[gaogaotiantian]

@Sefank is there a way to confirm the requested change is made before requesting a review? I'm seeing the review request but my change request is still there.

[Sefank]

@Sefank is there a way to confirm the requested change is made before requesting a review? I'm seeing the review request but my change request is still there.

I don't really know much about it. I guess that giving it a new review with approval would be ok and previous change-requested reviews just appear here as historical records?