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

Basic histogram performance counter implementation #2

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

victor-ludorum
Copy link
Owner

@victor-ludorum victor-ludorum commented Mar 22, 2018

Adding Histogram Performance Counter

This is a basic implementation of Histogram performance counter. Actually, I want to ask many things and they are co-related, therefore, I have opened this PR(for my private repository) so that I can update it what should be done for the proper implementation.
As it becomes reasonably correct I will open a PR for the HPX main branch master
@msimberg, @hkaiser Sir, Would you mind to have a review. 😊
Thanks in advance!!

@victor-ludorum victor-ludorum self-assigned this Mar 22, 2018
@victor-ludorum victor-ludorum added this to the 1.1.0 milestone Mar 22, 2018
@victor-ludorum victor-ludorum added the enhancement New feature or request label Mar 22, 2018
@victor-ludorum victor-ludorum added the help wanted Extra attention is needed label Mar 24, 2018
@msimberg
Copy link

Hi @victor-ludorum, thanks for putting this together.

I would suggest you do the following:

  • Squash your commits into a few (or one) coherent commits to make it easier for us to follow your changes.
  • Make sure this builds and runs correctly (may already be the case).
  • Add at least one test using the histogram counter (you will have to add more but at least one for now to show us that it works).
  • Open a PR already now against the main repository so that we can run all tests (but run relevant tests locally first).
  • Make sure you're not referring to any of the old code related to parcel coalescing (I spotted a few places).
  • Ask concrete questions if there is something you're wondering about (here or IRC).
  • I don't have any comments at this point on the implementation itself as it looks like it's mostly code from the parcel coalescing histogram counter (although if we're going to depend on the boost accumulators library I would still consider using their density counter).
  • The parcel coalescing histogram counter should be removed/replaced as this should provide the same functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants