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

Fix timing infrastructure #221

Closed
wants to merge 19 commits into from

Conversation

charmoniumQ
Copy link
Member

@charmoniumQ charmoniumQ commented Jan 24, 2021

Closes #208. Dependent on pending PRs for #220 and open_vins #2.

I have broken off the CPU timer into a separate repo here so that it can be reused between projects.

It will be the responsibility of a future PR to log that data to disk (issue #211).

This PR removes the old SQLite logging infrastructure. Logs that use or add-to the stack-frame's context should be added into the "extra info" of the CPU timer. Logs that do not should be converted to Switchboard topics and logged into a bag.

This PR is for the interface of the CPU timer. The reviewer should verify:

  • The interface is easy to use.
  • The documentation is sufficient enough for others to use the CPU timer.
  • No spurious changes were erroneously committed.
  • ILLIXR still runs at a normal speed and prints something like Got 75493 finished frames a few times (once for each thread) at the finish.

@charmoniumQ charmoniumQ changed the title Issue 209 cpu timer Fix logging infrastructure Jan 24, 2021
@charmoniumQ charmoniumQ marked this pull request as draft January 24, 2021 17:36
@charmoniumQ charmoniumQ changed the title Fix logging infrastructure Fix timing infrastructure Feb 8, 2021
@e3m3 e3m3 added this to High priority in ILLIXR improvements Mar 17, 2021
@e3m3 e3m3 moved this from High priority to Review in progress in ILLIXR improvements Mar 17, 2021
@charmoniumQ charmoniumQ moved this from Review in progress to High priority in ILLIXR improvements Mar 17, 2021
@e3m3 e3m3 force-pushed the issue-32-fix-mem-leak branch 2 times, most recently from 1b0b1a4 to 4e93aed Compare March 30, 2021 20:45
@e3m3 e3m3 force-pushed the issue-32-fix-mem-leak branch 3 times, most recently from 5ca32f6 to 9e36825 Compare April 16, 2021 01:38
@mvanmoer mvanmoer mentioned this pull request Aug 1, 2023
mvanmoer added a commit that referenced this pull request Aug 18, 2023
The top of file contains a large comment describing the timer
infrastructure. It references a test.sh which can be used to determine
the timing infrastructure overhead. This file exists only on the
project-scheduling branch. (Reminder - this PR was created from bits and
pieces of the project-scheduling and issue-209-cpu-timer branches, as
well as PR #221.) It is a BASH script which uses bazel. However, I can't locate
the actual tests. E.g. the first bazel command is

bazel run //include:cpu_timer_minimal_test \ # etc

ripgrep returns nothing for "cpu_timer_minimal_test" and find returns
there are no files with that file name.
ILLIXR improvements automation moved this from PR Backlog to Done Nov 2, 2023
@astro-friedel
Copy link
Contributor

Superceded by PR 383

astro-friedel pushed a commit that referenced this pull request Mar 27, 2024
The top of file contains a large comment describing the timer
infrastructure. It references a test.sh which can be used to determine
the timing infrastructure overhead. This file exists only on the
project-scheduling branch. (Reminder - this PR was created from bits and
pieces of the project-scheduling and issue-209-cpu-timer branches, as
well as PR #221.) It is a BASH script which uses bazel. However, I can't locate
the actual tests. E.g. the first bazel command is

bazel run //include:cpu_timer_minimal_test \ # etc

ripgrep returns nothing for "cpu_timer_minimal_test" and find returns
there are no files with that file name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants