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

QueryPerformanceRecorder: Changes for CorePlus Integration #4862

Merged
merged 5 commits into from
Nov 21, 2023

Conversation

nbauernfeind
Copy link
Member

No description provided.

@nbauernfeind nbauernfeind added core Core development tasks NoDocumentationNeeded NoReleaseNotesNeeded No release notes are needed. labels Nov 20, 2023
@nbauernfeind nbauernfeind added this to the November 2023 milestone Nov 20, 2023
@nbauernfeind nbauernfeind self-assigned this Nov 20, 2023
@nbauernfeind nbauernfeind marked this pull request as draft November 20, 2023 15:14
rcaudy
rcaudy previously approved these changes Nov 20, 2023
rcaudy
rcaudy previously approved these changes Nov 20, 2023
@cpwright
Copy link
Contributor

The performance queries should reintroduce Durations and an IntervalDuration as derived columns.

@nbauernfeind nbauernfeind marked this pull request as ready for review November 21, 2023 16:11
@nbauernfeind nbauernfeind changed the title Draft: QueryPerformanceRecorder: Changes for CorePlus Integration QueryPerformanceRecorder: Changes for CorePlus Integration Nov 21, 2023
Copy link
Member

@rcaudy rcaudy left a comment

Choose a reason for hiding this comment

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

This is good. I do want to raise that we're now calculating UPL interval duration by delta of wall-clock time, rather than delta of "monotonic" time. Are we sure we like this change?

@nbauernfeind nbauernfeind merged commit f679f0c into deephaven:main Nov 21, 2023
12 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 21, 2023
@nbauernfeind
Copy link
Member Author

Just to close the loop on the discussion, in case we ever look at this PR again:

We discussed offline that we're happy with duration being computed using wall-clock time given that usage nanos is the sum of [parallelized] System.nanoTime.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core Core development tasks NoDocumentationNeeded NoReleaseNotesNeeded No release notes are needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants