Skip to content

Update open once read many test#118

Open
briangow wants to merge 1 commit into
mainfrom
bg_open_once_update
Open

Update open once read many test#118
briangow wants to merge 1 commit into
mainfrom
bg_open_once_update

Conversation

@briangow
Copy link
Copy Markdown
Collaborator

@tcpan , as discussed I've updated the open once, read many test. This PR makes the following changes:

  • Removes the random channel test (this was deemed unnecessary since it doesn't make sense to compare the result of this test across formats given the random nature)
  • Simplifies the logic for selecting channels, now that the random test is not being run
  • Prints the sum of the open, read, close measurements as "total"
  • Makes printing of the open, read, close measurements themselves optional (only happens when verbose is set)
  • Updates the output to the summary file (to capture total cpu time)

Committer: Brian Gow <briangow@mit.edu>
@briangow briangow requested a review from tcpan December 23, 2024 20:03
@tcpan
Copy link
Copy Markdown
Contributor

tcpan commented Jan 2, 2025

I looked through this and the logic to summarize the time looks good.

Re. random channel:
The original purpose was to avoid caching skewing the measurements. However, as discussed with Brian, Benjamin, and Eric, this will allow different channel types to be mixed in the measurement. Two alternatives were discussed that do not involve random channel selection: 1. randomize the time window to read, and 2. read the same time window but exclude the first iteration of the run when computing median.

I will do some testing on this.

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.

2 participants