Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[switch] Number of Stat objects created #434

Closed
mcollina opened this issue Dec 13, 2018 · 4 comments
Closed

[switch] Number of Stat objects created #434

mcollina opened this issue Dec 13, 2018 · 4 comments

Comments

@mcollina
Copy link
Contributor

When running https://github.com/ipfs/benchmarks/blob/master/tests/local-transfer.js:

$ STAGE=local REMOTE=true FILESET=One64MBFile VERIFYOFF=true node local-transfer.js

That test uses only 2 peers.

I'm seeing around 44 Stats object being created, each one with their respective timer. Consider that at best (with libp2p/js-libp2p-switch#289), this means 44 events being fired every 2 seconds (the default). This is a lot of activity happening just to exists.

What is rationale behind keeping track of all those values? Are all these stats needed?

I've been looking at https://github.com/libp2p/js-libp2p-connection-manager/blob/0634c9116a3f708d3eb788ad7567022ec1f76195/src/index.js#L82, and the only Stats object that is needed is the global one https://github.com/libp2p/js-libp2p-switch/blob/f43084b93a1c048d68f7992357e3116029732ce2/src/stats/index.js#L40. Could the per-peer and per-transport stats be a) removed or b) put behind a flag?

@jacobheun
Copy link
Contributor

Wow yeah, that's a significant amount. I've been digging into this a bit to figure out where else stats are being used. I know we want to make updates to the Connection Manager to have it make more informed decisions about closing stale/idle/low priority connections in the near future, so per peer stats will become more important there.

It looks like bitswap is getting it's data from the stats object ipfs/js-ipfs#1198. I'm not sure if the per peer or protocol is being used at the moment. Perhaps @hacdias and/or @pgte can provide some insight on where and which stats items are being used?

I think it's worth investigating if we need the per protocol statistics and if not, make those opt in. We should also look into more efficient ways of calculating the averages over time, especially if we need the extra stats on by default.

@pgte
Copy link
Contributor

pgte commented Dec 14, 2018

I don't know of any place we're using per-protocol statistics. IRC it was discussed at the time with @kumavis the need of per-transport stats only: #175 (comment)
Not sure that's changed..

@kumavis
Copy link
Contributor

kumavis commented Apr 24, 2019

I ended up wanting really granular stats (transport/protocol usage per-peer) so I reused the Stats class but setup my own measurements

@daviddias daviddias changed the title Number of Stat objects created [switch] Number of Stat objects created Aug 22, 2019
@daviddias daviddias transferred this issue from libp2p/js-libp2p-switch Aug 22, 2019
maschad pushed a commit to maschad/js-libp2p that referenced this issue Jun 21, 2023
Bumps [@multiformats/multiaddr](https://github.com/multiformats/js-multiaddr) from 11.6.1 to 12.0.0.
- [Release notes](https://github.com/multiformats/js-multiaddr/releases)
- [Changelog](https://github.com/multiformats/js-multiaddr/blob/master/CHANGELOG.md)
- [Commits](multiformats/js-multiaddr@v11.6.1...v12.0.0)

---
updated-dependencies:
- dependency-name: "@multiformats/multiaddr"
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
maschad pushed a commit to maschad/js-libp2p that referenced this issue Jun 21, 2023
## [8.0.1](libp2p/js-libp2p-kad-dht@v8.0.0...v8.0.1) (2023-03-17)

### Dependencies

* bump @multiformats/multiaddr from 11.6.1 to 12.0.0 ([libp2p#434](libp2p/js-libp2p-kad-dht#434)) ([49dcc65](libp2p/js-libp2p-kad-dht@49dcc65))
@wemeetagain
Copy link
Member

We no longer have Stat objects, closing as stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants