bench: Move generated data to a dedicated translation unit #16299

pull promag wants to merge 1 commits into bitcoin:master from promag:2019-06-benchmark-data changing 5 files +44 −15
  1. promag commented at 2:16 PM on June 27, 2019: member

    With this change multiple benchmarks can use the same data without incurring in a bigger binary.

  2. fanquake added the label Tests on Jun 27, 2019
  3. promag cross-referenced this on Jun 27, 2019 from issue bench: Benchmark blockToJSON by fanatid
  4. promag force-pushed on Jun 27, 2019
  5. MarcoFalke commented at 2:56 PM on June 27, 2019: member

    Does not compile

  6. promag commented at 3:06 PM on June 27, 2019: member

    You must make clean or rm src/bench/data/block413567.raw.h.

  7. MarcoFalke commented at 3:10 PM on June 27, 2019: member

    Please see the travis output, there are missing files.

  8. promag force-pushed on Jun 27, 2019
  9. promag commented at 3:14 PM on June 27, 2019: member

    🤦‍♂ ... fixed.

  10. promag force-pushed on Jun 27, 2019
  11. MarcoFalke added the label Refactoring on Jun 27, 2019
  12. DrahtBot commented at 5:09 PM on June 27, 2019: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #16267 (bench: Benchmark blockToJSON by fanatid)
    • #16116 (Avoid unnecessary signing provider copies on descriptor expansion by Empact)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  13. promag force-pushed on Jun 27, 2019
  14. promag commented at 12:06 AM on June 28, 2019: member

    Not sure how to fix msvc build:

    c:\projects\bitcoin\src\bench\data\block413567.raw.h(1): error C2373: 'benchmark::data::block413567': redefinition; different type modifiers [C:\projects\bitcoin\build_msvc\bench_bitcoin\bench_bitcoin.vcxproj]
  15. in build_msvc/bench_bitcoin/bench_bitcoin.vcxproj:26 in 9a69bf9ac2 outdated 
      19 | @@ -20,6 +20,7 @@
  20 |      <ClCompile Include="..\..\src\bench\checkqueue.cpp" />
  21 |      <ClCompile Include="..\..\src\bench\coin_selection.cpp" />
  22 |      <ClCompile Include="..\..\src\bench\crypto_hash.cpp" />
  23 | +    <ClCompile Include="..\..\src\bench\data.cpp" />
  24 |      <ClCompile Include="..\..\src\bench\examples.cpp" />
  25 |      <ClCompile Include="..\..\src\bench\lockedpool.cpp" />
  26 |      <ClCompile Include="..\..\src\bench\mempool_eviction.cpp" />
    MarcoFalke commented at 3:09 AM on June 28, 2019: 
    diff --git a/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj b/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj
index 6ded6895cd..4831c4d514 100644
--- a/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj
+++ b/build_msvc/bench_bitcoin/bench_bitcoin.vcxproj
@@ -68,9 +68,9 @@
     <ItemGroup>
       <RawBenchFile Include="..\..\src\bench\data\*.raw" />
     </ItemGroup>
-    <HeaderFromHexdump RawFilePath="%(RawBenchFile.FullPath)" HeaderFilePath="%(RawBenchFile.FullPath).h" SourceHeader="static unsigned const char %(RawBenchFile.Filename)[] = {" SourceFooter="};" />
+    <HeaderFromHexdump RawFilePath="%(RawBenchFile.FullPath)" HeaderFilePath="%(RawBenchFile.FullPath).h" SourceHeader="const std::vector<uint8_t> %(RawBenchFile.Filename)[] = {" SourceFooter="};" />
   </Target>
   <Import Label="hexdumpTarget" Project="..\msbuild\tasks\hexdump.targets" />
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" />
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
-</Project>
\ No newline at end of file
+</Project>
    promag commented at 8:15 PM on June 30, 2019:

    I still can't find a way to fix it 😕

  16. promag force-pushed on Jun 28, 2019
  17. promag force-pushed on Jun 28, 2019
  18. promag force-pushed on Jun 29, 2019
  19. promag force-pushed on Jun 30, 2019
  20. MarcoFalke commented at 12:12 PM on July 1, 2019: member

    The windows build just fails without any error message:

    C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.CppCommon.targets(423,5): error MSB6006: "clcache.exe" exited with code 1. [C:\projects\bitcoin\build_msvc\bench_bitcoin\bench_bitcoin.vcxproj]

    @sipsorcery Any ideas?

  21. sipsorcery commented at 7:10 PM on July 1, 2019: member

    @MarcoFalke the almost 1 million byte vector initialiser list kills the Microsoft C++ compiler. The Visual Studio 2017 version crashes when compiling data.cpp nce the initialiser list gets close to 80k items. The Visual Studio 2019 version didn't crash for with the full 1 million items within approx 5 minutes at which point I killed it.

    There is no useful error message or documentation I could find as to why the compiler doesn't like initialiser lists.

    What I can verify is that what causes the problem is switching from: static unsigned const char block413567[] to extern const std::vector<uint8_t> block413567;

    Compiling with a 1 million element array works well and is quick. With the vector cl.exe either cannot cope or takes an inordinate amount of time. I'd recommend sticking with the array for such a large amount of data.

  22. in src/bench/data.h:8 in e0bf71d232 outdated 
       0 | @@ -0,0 +1,21 @@
   1 | +// Copyright (c) 2019 The Bitcoin Core developers
   2 | +// Distributed under the MIT software license, see the accompanying
   3 | +// file COPYING or http://www.opensource.org/licenses/mit-license.php.
   4 | +
   5 | +#ifndef BITCOIN_BENCH_DATA_H
   6 | +#define BITCOIN_BENCH_DATA_H
   7 | +
   8 | +#include <bench/data.h>
    sipsorcery commented at 7:11 PM on July 1, 2019:

    This looks like a recursive include?

  23. promag commented at 7:32 PM on July 1, 2019: member

    @sipsorcery thanks! I'll work around it.

  24. promag force-pushed on Jul 1, 2019
  25. promag commented at 9:36 PM on July 1, 2019: member

    @MarcoFalke updated, msvc is happy now, @sipsorcery thanks a lot!

  26. in src/bench/data.h:14 in d77899f266 outdated 
       9 | +#include <vector>
  10 | +
  11 | +namespace benchmark {
  12 | +namespace data {
  13 | +
  14 | +extern const std::vector<uint8_t>& block413567;
    MarcoFalke commented at 3:30 PM on July 2, 2019: 
    extern const std::vector<uint8_t> block413567;

    We generally do it without the &, no?

  27. MarcoFalke commented at 3:31 PM on July 2, 2019: member

    Fine with me. While we don't ship the bench binary and have no space concerns, this shouldn't hurt either.

  28. promag force-pushed on Jul 2, 2019
  29. bench: Move generated data to a dedicated translation unit 3d60a03a7c
  30. promag force-pushed on Jul 2, 2019
  31. laanwj commented at 1:39 PM on July 3, 2019: member

    Fine with me. While we don't ship the bench binary and have no space concerns, this shouldn't hurt either.

    Yes, I think this is good habit. And might also reduce total parsing/compile time.

    code review ACK 3d60a03a7cfb2d46b5f10633e9f6a9a36b8cb76f

  32. MarcoFalke referenced this in commit 91c345eb92 on Jul 3, 2019
  33. MarcoFalke merged this on Jul 3, 2019
  34. MarcoFalke closed this on Jul 3, 2019
  35. promag deleted the branch on Jul 3, 2019
  36. deadalnix referenced this in commit 33b30bc21b on May 6, 2020
  37. furszy cross-referenced this on Jun 11, 2021 from issue Road to Tor v3 support (BIP155) by furszy
  38. furszy cross-referenced this on Jul 15, 2021 from issue [Build] bench build, raw generated file improvement and fixes. by furszy
  39. random-zebra referenced this in commit 26e90b014e on Jul 21, 2021
  40. random-zebra referenced this in commit b4751e10ce on Aug 11, 2021
  41. kwvg referenced this in commit acf58ff0b0 on Nov 6, 2021
  42. kwvg referenced this in commit d920018f06 on Nov 30, 2021
  43. kwvg referenced this in commit 770b697da4 on Dec 3, 2021
  44. kwvg referenced this in commit 7ab0e85ee3 on Dec 4, 2021
  45. kwvg referenced this in commit a273bec000 on Dec 6, 2021
  46. kwvg referenced this in commit 55e0d56592 on Dec 8, 2021
  47. kwvg referenced this in commit f2ff3e442f on Dec 8, 2021
  48. kwvg referenced this in commit 6ed835cbda on Dec 8, 2021
  49. kwvg referenced this in commit ac996a545d on Dec 11, 2021
  50. kwvg referenced this in commit fbbe59068c on Dec 12, 2021
  51. kwvg referenced this in commit a1c6460c97 on Dec 12, 2021
  52. kwvg referenced this in commit 6a6a262ba7 on Dec 12, 2021
  53. kwvg referenced this in commit b0cc1e2772 on Dec 12, 2021
  54. kwvg referenced this in commit 195772068e on Dec 12, 2021
  55. kwvg referenced this in commit cd23b0f8e8 on Dec 12, 2021
  56. kwvg referenced this in commit e51c4c41c0 on Dec 12, 2021
  57. bitcoin locked this on Dec 16, 2021
Contributors
promagMarcoFalkeDrahtBotsipsorcerylaanwj
Labels
RefactoringTests
Linked (view graph)
#16267 bench: Benchmark blockToJSON
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:54 UTC