dtn7 / dtn7-gold

Delay-tolerant networking software suite, Bundle Protocol Version 7 (deprecated)
https://dtn7.github.io/
Other
78 stars 14 forks source link

Bundle Builder significantly slower than manual bundle creation #4

Closed gh0st42 closed 5 years ago

gh0st42 commented 5 years ago

Using the builder to construct a new bundle is significantly slower than doing it manually.

Creating 100000 bundles with 0 using builder:   37285 bundles/second
Creating 100000 bundles with 1 using builder:   35019 bundles/second
Creating 100000 bundles with 2 using builder:   35505 bundles/second
Creating 100000 bundles with 0:     51881 bundles/second
Creating 100000 bundles with 1:     51950 bundles/second
Creating 100000 bundles with 2:     51148 bundles/second
Loading 100000 bundles with 0:  68270 bundles/second
Loading 100000 bundles with 1:  68214 bundles/second
Loading 100000 bundles with 2:  68319 bundles/second

0: CRCNo 1: CRC16 2: CRC32

The benchmarking code is attached, rename to `benchmark.go`` benchmark.txt

oxzi commented 5 years ago

This benchmark is not quite accurate. Your bench_bundle_create function to benchmark the simple bundle creation gets a CRC type passed, but does nothing with it except printing it in a log. Furthermore, the two functions will create different bundles. For comparison, one should take a look at the TestBundleBuilderSimple function in bundle/bundle_builder_test.go.

However, the BundleBuilder is the idea of a more intuitive way to create bundles and is not even in use. There needs to be some work done, before I would consider replacing the old way to create bundles with the builder.

Whatsoever, thanks a lot for taking a look at this code and inspecting it!

gh0st42 commented 5 years ago

Correct, , i forgot to add SetCRCType to the non-builder version and call CalculateCRC, maybe ToCbor should do that in case no crc sum has been calculated yet. Also it was not clear that the builder calculated CRC automatically and the other one doesn't - should probably be mentioned in the docs.

After changing "ABC" to "AAA" both versions also produce the exact same encoded bundle, the strings shouldn't make a speed difference as they both have the same length.

The new numbers with crc calculation and not creation time now but dtn epoch:

Creating 100000 bundles with 0 using builder:   37950 bundles/second
Creating 100000 bundles with 1 using builder:   36106 bundles/second
Creating 100000 bundles with 2 using builder:   35887 bundles/second
Creating 100000 bundles with 0:         39336 bundles/second
Creating 100000 bundles with 1:         36447 bundles/second
Creating 100000 bundles with 2:         35215 bundles/second
Encoding 100000 bundles with 0:         192249 bundles/second
Encoding 100000 bundles with 1:         171602 bundles/second
Encoding 100000 bundles with 2:         173284 bundles/second
Loading 100000 bundles with 0:          68931 bundles/second
Loading 100000 bundles with 1:          42892 bundles/second
Loading 100000 bundles with 2:          41556 bundles/second

Here builder and non-builder versions almost perform the same. Thanks for the clarification!

oxzi commented 5 years ago

Well, the NewBundle function offers no option to specify a CRC. There is the SetCRCType method for the Bundle type, which notifies the user about the CalculateCRC method. The BundleBuilder is an idea to address this confusing usage pattern.

Thanks a lot for recalculating the benchmarks and having a look at the code.