[ntuple] Add new performance counters to `RPageSink{File,Daos}` by jalopezg-git · Pull Request #8328 · root-project/root

jalopezg-git · 2021-06-03T15:51:39Z

This pull-request adds new write performance counters to the file/DAOS backends:

fSzWritePayload: that keeps track of the total volume written in committed pages.
fSzZip: volume before zipping
fTime{Wall,Cpu}Zip: that measure the wall clock/cpu time spent compressing.

This suffices to compute the actual write throughput, where needed.

Closes issue #8283.

phsft-bot · 2021-06-03T15:51:54Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

mxxo

Hey Javier, looks good to me! Just saw some typos. Maybe ("szZip", "B", "volume before zipping") makes sense to add as well for symmetry with the page source?

root/tree/ntuple/v7/src/RPageStorageFile.cxx

Line 221 in 6832151

    
           *fMetrics.MakeCounter<RNTupleAtomicCounter*>("szUnzip", "B", "volume after unzipping"),

tree/ntuple/v7/src/RPageStorageDaos.cxx

tree/ntuple/v7/src/RPageStorageFile.cxx

jalopezg-git · 2021-06-03T21:02:00Z

Hey Javier, looks good to me! Just saw some typos. Maybe ("szZip", "B", "volume before zipping") makes sense to add as well for symmetry with the page source?

root/tree/ntuple/v7/src/RPageStorageFile.cxx

Line 221 in 6832151

*fMetrics.MakeCounter<RNTupleAtomicCounter*>("szUnzip", "B", "volume after unzipping"),

Thanks for reviewing @mxxo! +1 for that or not, @jblomer?

jblomer · 2021-06-03T21:31:57Z

Thanks for reviewing @mxxo! +1 for that or not, @jblomer?

+1

phsft-bot · 2021-06-04T11:06:43Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

phsft-bot · 2021-06-04T11:06:56Z

Build failed on ROOT-performance-centos8-multicore/default.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

jalopezg-git · 2021-06-04T11:09:04Z

Thanks for reviewing @mxxo! +1 for that or not, @jblomer?

+1

Done. :-)

jalopezg-git · 2021-06-08T10:03:54Z

@phsft-bot build

phsft-bot · 2021-06-08T10:04:08Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

jblomer

Very well! I think we are almost there.

tree/ntuple/v7/src/RPageStorageDaos.cxx

tree/ntuple/v7/src/RPageStorageFile.cxx

phsft-bot · 2021-06-08T15:21:05Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

jblomer

LGTM, just one more warning needs removal

tree/ntuple/v7/src/RPageStorageDaos.cxx

phsft-bot · 2021-06-08T16:26:02Z

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft11.dyndns.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Warnings:

[2021-06-08T15:50:18.494Z] /home/sftnight/build/workspace/root-pullrequests-build/root/tree/ntuple/v7/src/RPageStorageDaos.cxx:169:19: warning: unused parameter ‘columnId’ [-Wunused-parameter]

phsft-bot · 2021-06-08T17:31:10Z

Build failed on mac11.0/cxx17.
Running on macphsft20.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Warnings:

[2021-06-08T16:39:08.640Z] /Users/sftnight/build/workspace/root-pullrequests-build/root/tree/ntuple/v7/src/RPageStorageDaos.cxx:169:19: warning: unused parameter 'columnId' [-Wunused-parameter]

phsft-bot · 2021-06-08T18:05:09Z

Build failed on windows10/cxx14.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Errors:

[2021-06-08T18:05:03.243Z] LINK : fatal error LNK1104: cannot open file 'C:\build\workspace\root-pullrequests-build\build\bin\libCore.dll' [C:\build\workspace\root-pullrequests-build\build\core\Core.vcxproj]

Several new counters were added to the file backend: - fSzWritePayload: that keeps track of the total volume written in commited pages. - fSzZip: total size of data before zipping - fTime{Wall,Cpu}Zip: that measure the wall clock/cpu time spent compressing. This suffices to compute the actual write throughput, where needed.

Port of the available performance counters for RPageSinkFile to the DAOS backend. Note however, that common performance metrics might be probably handled by the base class; to be discussed.

phsft-bot · 2021-06-08T18:18:38Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

jalopezg-git added improvement in:RNTuple labels Jun 3, 2021

jalopezg-git requested review from jblomer and mxxo June 3, 2021 15:51

jalopezg-git self-assigned this Jun 3, 2021

jalopezg-git requested a review from pcanal as a code owner June 3, 2021 15:51

jalopezg-git linked an issue Jun 3, 2021 that may be closed by this pull request

[rntuple] Add performance counters to classes derived from RPageSink #8283

Closed

mxxo reviewed Jun 3, 2021

View reviewed changes

tree/ntuple/v7/src/RPageStorageDaos.cxx Outdated Show resolved Hide resolved

tree/ntuple/v7/src/RPageStorageFile.cxx Outdated Show resolved Hide resolved

jalopezg-git force-pushed the jalopezg-rntuple-sinkcounters branch from 954f5a7 to bb5a3a2 Compare June 4, 2021 11:06

jblomer requested changes Jun 8, 2021

View reviewed changes

tree/ntuple/v7/src/RPageStorageDaos.cxx Outdated Show resolved Hide resolved

tree/ntuple/v7/src/RPageStorageFile.cxx Outdated Show resolved Hide resolved

jalopezg-git force-pushed the jalopezg-rntuple-sinkcounters branch from bb5a3a2 to 394bf1c Compare June 8, 2021 15:20

jblomer requested changes Jun 8, 2021

View reviewed changes

tree/ntuple/v7/src/RPageStorageDaos.cxx Outdated Show resolved Hide resolved

jalopezg-git added 2 commits June 8, 2021 20:15

[ntuple,daos] Add performance counters to RPageSinkDaos

a46758a

Port of the available performance counters for RPageSinkFile to the DAOS backend. Note however, that common performance metrics might be probably handled by the base class; to be discussed.

jalopezg-git force-pushed the jalopezg-rntuple-sinkcounters branch from 394bf1c to a46758a Compare June 8, 2021 18:18

jblomer approved these changes Jun 8, 2021

View reviewed changes

jalopezg-git merged commit 490710c into root-project:master Jun 9, 2021

jalopezg-git deleted the jalopezg-rntuple-sinkcounters branch June 9, 2021 11:27

Conversation

jalopezg-git commented Jun 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phsft-bot commented Jun 3, 2021

Uh oh!

mxxo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jalopezg-git commented Jun 3, 2021

Uh oh!

jblomer commented Jun 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phsft-bot commented Jun 4, 2021

Uh oh!

phsft-bot commented Jun 4, 2021

Uh oh!

jalopezg-git commented Jun 4, 2021

Uh oh!

jalopezg-git commented Jun 8, 2021

Uh oh!

phsft-bot commented Jun 8, 2021

Uh oh!

jblomer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

phsft-bot commented Jun 8, 2021

Uh oh!

jblomer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phsft-bot commented Jun 8, 2021

Warnings:

Uh oh!

phsft-bot commented Jun 8, 2021

Warnings:

Uh oh!

phsft-bot commented Jun 8, 2021

Errors:

Uh oh!

phsft-bot commented Jun 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jalopezg-git commented Jun 3, 2021 •

edited

Loading

jblomer commented Jun 3, 2021 •

edited

Loading