Skip to content

Conversation

@pravblockc
Copy link

@pravblockc pravblockc commented Aug 10, 2021

Backports v0.18: PR's bitcoin#14242, 15351 and 14519

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should drop 14242 (it's empty) + see below (feels like quite a few earlier PRs are missing)

@pravblockc
Copy link
Author

should drop 14242 (it's empty) + see below (feels like quite a few earlier PRs are missing)

Okay, I will take-up earlier PR's that are mandatory to port this(will fix this later)

@github-actions
Copy link

This pull request has conflicts, please rebase.

@github-actions
Copy link

This pull request has conflicts, please rebase.

@pravblockc pravblockc force-pushed the backports-v0.18-pr9 branch from 7e44478 to eab19ff Compare August 17, 2021 01:14
@pravblockc pravblockc changed the title Backports v0.18: PR's #14242, 15351, 14519 and 14982 Backports v0.18: PR's #14242, 15351 and 14519 Aug 17, 2021
@pravblockc pravblockc force-pushed the backports-v0.18-pr9 branch from eab19ff to 9341b3c Compare August 17, 2021 02:41
@pravblockc
Copy link
Author

should drop 14242 (it's empty) + see below (feels like quite a few earlier PRs are missing)

added commit that was missing for 14242

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pls see below + should drop an empty test/sanitizer_suppressions/ubsan file from 14242

@pravblockc pravblockc force-pushed the backports-v0.18-pr9 branch from 9341b3c to 02bbb42 Compare August 18, 2021 22:26
@UdjinM6
Copy link

UdjinM6 commented Aug 18, 2021

Pls rebase on top of develop and add missing changes to test/sanitizer_suppressions/ubsan (14242), this file should exist now that we merged 14764 via #4341.

MarcoFalke and others added 3 commits August 18, 2021 22:24
…mance with perf

13782b8 docs: add perf section to developer docs (James O'Beirne)
58180b5 tests: add utility to easily profile node performance with perf (James O'Beirne)

Pull request description:

  Adds a context manager to easily (and selectively) profile node performance during functional test execution using `perf`.

  While writing some tests, I encountered some odd bitcoind slowness. I wrote up a utility (`TestNode.profile_with_perf`) that generates performance diagnostics for a node by running `perf` during the execution of a particular region of test code.

  `perf` usage is detailed in the excellent (and sadly unmerged) bitcoin#12649; all due props to @eklitzke.

  ### Example

  ```python
  with node.profile_with_perf("large-msgs"):
      for i in range(200):
          node.p2p.send_message(some_large_msg)
      node.p2p.sync_with_ping()
  ```

  This generates a perf data file in the test node's datadir (`/tmp/testtxmpod0y/node0/node-0-TestName-large-msgs.perf.data`).

  Running `perf report` generates nice output about where the node spent most of its time while running that part of the test:

  ```bash
  $ perf report -i /tmp/testtxmpod0y/node0/node-0-TestName-large-msgs.perf.data --stdio \
    | c++filt \
    | less

  # To display the perf.data header info, please use --header/--header-only options.
  #
  #
  # Total Lost Samples: 0
  #
  # Samples: 135  of event 'cycles:pp'
  # Event count (approx.): 1458205679493582
  #
  # Children      Self  Command          Shared Object        Symbol
  # ........  ........  ...............  ...................  ........................................................................................................................................................................................................................................................................
  #
      70.14%     0.00%  bitcoin-net      bitcoind             [.] CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)
                  |
                  ---CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)

      70.14%     0.00%  bitcoin-net      bitcoind             [.] CNetMessage::readData(char const*, unsigned int)
                  |
                  ---CNetMessage::readData(char const*, unsigned int)
                     CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)

      35.52%     0.00%  bitcoin-net      bitcoind             [.] std::vector<char, zero_after_free_allocator<char> >::_M_fill_insert(__gnu_cxx::__normal_iterator<char*, std::vector<char, zero_after_free_allocator<char> > >, unsigned long, char const&)
                  |
                  ---std::vector<char, zero_after_free_allocator<char> >::_M_fill_insert(__gnu_cxx::__normal_iterator<char*, std::vector<char, zero_after_free_allocator<char> > >, unsigned long, char const&)
                     CNetMessage::readData(char const*, unsigned int)
                     CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)

  ...
  ```

Tree-SHA512: 9ac4ceaa88818d5eca00994e8e3c8ad42ae019550d6583972a0a4f7b0c4f61032e3d0c476b4ae58756bc5eb8f8015a19a7fc26c095bd588f31d49a37ed0c6b3e
7fdb92e Update linearize-hashes.py (OverlordQ)

Pull request description:

  Fix class case issue.

Tree-SHA512: 42d26e38b75b6b419ae4a9ca5c110d4ced0f7c5db997a64c8ab5dfc25dc228008349b6423c20ef4e396a773ff31f1f3f0092331c5e89748216e253e4d8337e9a
…t(nullptr, 0, 0)) if an invalid string is passed to DecodeSecret(...)

d855e4c Avoid triggering undefined behaviour (std::memset(nullptr, 0, 0)) if an invalid string is passed to DecodeSecret(...) (practicalswift)

Pull request description:

  Avoid triggering undefined behaviour (`std::memset(nullptr, 0, 0)`) if an invalid string is passed to `DecodeSecret(...)`.

  Background reading: [memcpy (and friends) with NULL pointers](https://www.imperialviolet.org/2016/06/26/nonnull.html)

  Steps to reproduce:

  ```
  ./configure --with-sanitizers=undefined && make check && ./test/functional/test_runner.py
  ```

Tree-SHA512: b8325ced4f724d9c03065e0747af56b1f297a90d9fb09a24d46c3231a90dce3df6299f2c41f863b5cec18eaeded7b46ee4b93d9a52adc2541eb4c44d2c0965d9
@pravblockc pravblockc force-pushed the backports-v0.18-pr9 branch from 02bbb42 to 7ac0213 Compare August 19, 2021 01:38
@pravblockc
Copy link
Author

Pls rebase on top of develop and add missing changes to test/sanitizer_suppressions/ubsan (14242), this file should exist now that we merged 14764 via #4341.

above mentioned changes has been done

@UdjinM6 UdjinM6 added this to the 18 milestone Aug 19, 2021
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

utACK

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK for merging via merge commit

@PastaPastaPasta PastaPastaPasta merged commit c469721 into dashpay:develop Aug 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants