-
Notifications
You must be signed in to change notification settings - Fork 54
Avoid allocating small views at runtime inside Field methods #3021
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
Conversation
These were likely residues from when FieldLayout did not store device-friendly extents
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Using Repos:
Pull Request Author: bartgol |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
SCREAM_PullRequest_Autotester_Weaver # 6087 FAILED (click to see last 100 lines of console output)
SCREAM_PullRequest_Autotester_Mappy # 5858 FAILED (click to see last 100 lines of console output)
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Using Repos:
Pull Request Author: bartgol |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
SCREAM_PullRequest_Autotester_Weaver # 6090 FAILED (click to see last 100 lines of console output)
SCREAM_PullRequest_Autotester_Mappy # 5860 FAILED (click to see last 100 lines of console output)
|
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
Using Repos:
Pull Request Author: bartgol |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: SCREAM_PullRequest_Autotester_Weaver
Jenkins Parameters
Build InformationTest Name: SCREAM_PullRequest_Autotester_Mappy
Jenkins Parameters
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
All Jobs Finished; status = PASSED, target_sha=a269ef91a3da65ee4e23f4e33736684366046b4d, However Inspection must be performed before merge can occur... |
@ndkeen, recall these are the (small) mallocs in the IO, should we perf-prof? (removing automerge label until we check the perf) |
Up to you if you want to check performance, but given how busy we are, and given that if you look at the code changes you can be fairly sure that there is no perf regression. I wouldn't hold off the merge until perf is checked, especially since we're not talking about 2x speedup/slowdown here. We can't start (manually) perf-checking every single PR, imho. My 2 cents. If you guys want to run perf checks, feel free to do so. |
I think we are only interested in checking things we came across during the hackathon, that's all... I agree this is not worth checking, but I will defer to @ndkeen to decide :) The side goal is to document perf improvements related to NESAP/Hackathon/etc. for better documentation |
Status Flag 'Pull Request AutoTester' - AutoMerge IS ENABLED, but the Label AT: AUTOMERGE is not set. Either set Label AT: AUTOMERGE or manually merge the PR... |
Might be good to note that we found cuda malloc/frees during file output section -- specifically vertical interpolation. |
For the timer |
These were likely residues from when FieldLayout did not store device-friendly extents.
I had this branch sitting on my workstation, but forgot to open the PR. These small device allocs were found during the NERSC hackathon a few weeks back.
fixes #2958