Reduce the number of PIO sync during IOStream write#463
Conversation
|
@grnydawn This seems to be working for my column testing. It's getting far more than 30% reduction. Here is the original timing from the PIO section here is with your changes I see a massive reduction in the number of sync calls (as I think is expected), write totals go down in many places. I now wonder if some of the improvement is due to other changes in my rebase. Let me test again without this set of changes but with all the rebase changes. |
|
Here is with the updated code, but not your PR So the large timing reduction is indeed the changes in this PR. This is all on pm-cpu. Are there other tests you'd like me to run. And I didn't say this explicitly but I also confirmed the output data is unchanged with this PR. |
|
Sorry disregard my last comment, there is something wrong with the output. Looking into it to see what the cause is |
|
@grnydawn thanks for looking into this. As the comment suggested, the sync was there for safety and tests were failing without it, probably because of the Decomp issue that you found. I'm still maybe a little worried about the general PIO buffers, especially if we remove the decomp computation that's adding some overhead and allowing time for buffers to clear. I guess we'll see what Luke's testing finds. I'll use a separate comment for the Decomp caching. |
|
On the decomp caching. One of the reasons I recomputed every time is that it requires storing address offset arrays for each unique array size. At a minimum, it would require 3 full-sized arrays to manage each of the mesh locations (and a similar set for 2-d fields - tho maybe we could just extract the k=0 slice from the full-sized decomp). And then it would need additional Decomps for any array that didn't conform to the generic sizes (eg NCells x NVertLayers). I suspect we could precompute decomps for the generic sizes and only compute new ones for the outliers. That wouldn't be too large a cost. |
@philipwjones , I am learning the IOStream implementation while debugging, so there could be some misunderstanding on my part. My understanding is that the key issue was deleting the decomposition after every field write while the PIO buffer still had buffered data. The sync operation forces the data to be flushed to the file before tearing the decomposition down. Yes, let’s wait to see what Luke finds. |
This approach of precalculating the common sizes and only computing new ones when necessary seems like the right direction to me. |
|
Okay sorry for the slow return to this. I had some issues to work through in my codebase. I was able to test again with reasonable results. From what I can tell with this PR and without there are no diffs in the output. This could be a consequence of the configuration I'm testing (16 cell planar), so should probably take results with a grain of salt |
@vanroekel , Good to know that you got the same output. So far, I have tested this PR only with the Omega ctests on PM-CPU, PM-GPU, and Chrysalis. I think the |
3 participants
This PR reduces the number of PIO sync operations performed during IOStream writes for field data.
Instead of executing a PIO sync for each field variable, it synchronizes the physical file only when the file is closed.
All CTest tests pass on pm-gpu, pm-cpu, and chrysalis.
The
IOSTREAM_TESTon pm-cpu is approximately 30% faster than the original implementation.This PR provides a partial fix for this issue. Additional performance optimizations may be achieved by implementing caching and reuse of decompositions.
Checklist
Testingwith the following: