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

Applying latest ROOT commits to branch V6-26-00-patches #184

Conversation

aandvalenzuela
Copy link

No description provided.

pcanal and others added 11 commits May 23, 2023 17:35
Previously depending whether the TTreeCache was globally enabled or disabled and whether fAutoFlush was set or not,
the TTreeCache would be enabled or not.

In particular no longer rely on TTree::Streamer setting fCacheSize to an estimated value when the TTreeCache
is disabled globally (The old code lead to the cache being enabled even-though it was disabled).

Note/Reminder: `TTreePlayer::Process` (and hence `TTree::Draw`) does not rely on the global setting on
whether the cache should be used or not and only respect an explicit disabling of the cache for the
specific `TTree` being used.
Prior to this change, the cache of which basket to start the search
on would restart from the first basket at each cluster iteration
(i.e. for filling the cache with with clusters)

On an extreme example:
        15,272,928 entries
           152,739 baskets (and as many clusters)
            10,000 Actual TTreeCache buffer size (minimum allowed)
             8,442 estimated buffer size of TTreeCache (1.5 times compressed buffer size)
               400 bytes per baskets
               100 entries per baskets (i.e. per clusters)
                25 number of cluster per TTreeCache buffer for single branch with default size.
                 1 float per entry (reading a single branch).

This ends up repairing the performance of a simple `TTree::Draw` of a single branch
from 1 hour back down to 7s (performance seem in v6.12).

This correct an issue introduced by commit 73f6223 first since in v6.14/00

This fixes #12649
…ket.

    On an extreme example:
    15,272,928 entries
       152,739 baskets (and as many clusters)
        10,000 Actual TTreeCache buffer size (minimum allowed)
         8,442 estimated buffer size of TTreeCache (1.5 times compressed buffer size)
           400 bytes per baskets
           100 entries per baskets (i.e. per clusters)
            25 number of cluster per TTreeCache buffer for single branch with default size.
             1 float per entry (reading a single branch).

    we gain 20% run time on a call to `Tree::Draw`
(cherry picked from commit d05fe48)
(cherry picked from commit e92a604)
There can be multiple attributes in the forward declaration, see the
added test in roottest/cling/dict/enum (reduced from a case reported
by CMS in cms-sw/cmssw#42234), so we have
to look for the last closing parentheses.

(cherry picked from commit 9d2f761)
@aandvalenzuela aandvalenzuela changed the title Applying lates ROOT commits to branch V6-26-00-patches Applying latest ROOT commits to branch V6-26-00-patches Aug 2, 2023
@cmsbuild
Copy link

cmsbuild commented Aug 2, 2023

A new Pull Request was created by @aandvalenzuela (Andrea Valenzuela) for branch cms/v6-26-00-patches/2f336483c6.

@cmsbuild, @smuzaffar, @aandvalenzuela, @iarspider can you please review it and eventually sign? Thanks.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.
cms-bot commands are listed here

@aandvalenzuela aandvalenzuela merged commit 36d8b2b into cms-sw:cms/v6-26-00-patches/2f336483c6 Aug 2, 2023
@smuzaffar
Copy link

why this PR @aandvalenzuela ?

@aandvalenzuela
Copy link
Author

I created the branch cms/v6-26-00-patches/2f336483c6 in cms-sw/root from the lastest v6.26 branch in our repo (https://github.com/cms-sw/root/tree/cms/v6-26-00-patches/a25c523160) and applied the commits from the v6.26 branch in root-project/root (https://github.com/root-project/root/tree/v6-26-00-patches)

@smuzaffar
Copy link

did our normal proceedure to update root not work for this? I mean running https://github.com/cms-sw/cms-bot/blob/master/cmsdist-tool-update.sh for IB/CMSSW_13_3_X/master ?

@smuzaffar
Copy link

I am deleting cms/v6-26-00-patches/2f336483c6 branch now and please use the cmsdist-tool-update.sh for updating

@aandvalenzuela
Copy link
Author

It was creating a cms/master/XXXXXXXXXX branch instead of cms/v6-26-00-patches/XXXXXXXXXX

@aandvalenzuela
Copy link
Author

Okay, let me retry! Thanks

@smuzaffar
Copy link

It was creating a cms/master/XXXXXXXXXX branch instead of cms/v6-26-00-patches/XXXXXXXXXX

you might have used wrong cmsdist branch. if you have used IB/CMSSW_13_3_X/master and script was creating cms/master/XXXXXXXXXX then we should fix the script

@aandvalenzuela
Copy link
Author

You are right, sorry for the noise!

@aandvalenzuela
Copy link
Author

See cms-sw/cmsdist#8630

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants