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

[cling] Memory hogging when checking if type is an enum #10454

Open
etejedor opened this issue Apr 26, 2022 · 13 comments
Open

[cling] Memory hogging when checking if type is an enum #10454

etejedor opened this issue Apr 26, 2022 · 13 comments
Assignees

Comments

@etejedor
Copy link
Contributor

etejedor commented Apr 26, 2022

Initially reported here:

https://root-forum.cern.ch/t/pyroot-and-std-vector-dramatic-ram-usage/49711

when a user was getting a vector<int> branch from a tree in a loop in PyROOT.

After a bit of digging, it seems the culprit is a call that is made from cppyy to TInterpreter::ClassInfo_IsEnum. The following is a C++ reproducer (tested with current master):

float getMem() {
        auto info = ProcInfo_t();
        gSystem->GetProcInfo(&info);
        float mem = (float)info.fMemResident;
        return mem*1e-3;
}

void test()
{
    for (int i=0; i < 100000; ++i) {
        gInterpreter->ClassInfo_IsEnum("vector<int>");
        if (i%1000 == 0)
            printf("RSS at iteration %d is %f\n", i, getMem());
    }
}

When running the macro above, it can be seen how the memory usage increases after each iteration.

@eguiraud
Copy link
Member

Related (or maybe the exact same issue): #9029

@pcanal
Copy link
Member

pcanal commented Apr 26, 2022

I don't know if it is an option at that place in the code but calling a combination of
TClass::GetClass(name_of_type);
and
TEnum::GetEnum(name_of_type);
should be more memory efficient.

@aescalante
Copy link

Dear experts, @etejedor,

I think I am facing a similar memory usage problem in PyROOT (ROOT 6.24/06).

Do you know if this issue has been fixed? If yes, in which release?

Thanks a lot!
Cheers,
Alberto

@etejedor
Copy link
Contributor Author

Dear Alberto,

I'm afraid this is not yet fixed, but @Axel-Naumann can confirm.

@Axel-Naumann
Copy link
Member

@etejedor #10454 (comment) suggests a change in PyROOT. Can you confirm whether that helps?

@etejedor
Copy link
Contributor Author

So iiuc the suggestion is to replace this line by something like return TEnum::GetEnum(tn_short.c_str());? Where should TClass::GetClass be used here and for what purpose?

@Axel-Naumann
Copy link
Member

Axel-Naumann commented Sep 21, 2022

Something along the lines of

    if (TClass::GetClass(tn_short.c_str()) return false;
    return gInterpreter->ClassInfo_IsEnum(tn_short.c_str());

maybe?

@etejedor
Copy link
Contributor Author

Thanks @Axel-Naumann , so TClass::GetClass is used to discard classes faster instead of just going directly for TEnum::GetEnum? (I assume the second line in your code snippet meant to use TEnum::GetEnum and not the current ClassInfo_IsEnum, which seems to be behind the memory hogging).

@Axel-Naumann
Copy link
Member

Right - TEnum::GetEnum() is doing these things already, unlike ClassInfo_IsEnum(). So just using TEnum::GetEnum() instead of ClassInfo_IsEnum() would be enough!

etejedor added a commit to etejedor/root that referenced this issue Sep 22, 2022
As reported in issue root-project#10454, calling ClassInfo_IsEnum() repeatedly
increases the memory consumption of the program. This does not
happen with TEnum::GetEnum, which can be used instead.
@etejedor
Copy link
Contributor Author

TEnum::GetEnum() does not seem to suffer from the same issue. #11412 replaces the use of ClassInfo_IsEnum() by TEnum::GetEnum() in clingwrapper so there is no memory hogging in PyROOT for this case.

@etejedor
Copy link
Contributor Author

#11412 revealed a test failure caused by the switch to TEnum::GetEnum. This is a reproducer in C++, extracted from the test that fails:

enum EFruit {kApple=78, kBanana=29, kCitrus=34};

void repro()
{
    auto type = "std::vector<EFruit>::value_type";
    cout << gInterpreter->ClassInfo_IsEnum(type) << endl;
    cout << TEnum::GetEnum(type) << endl;
}

which prints

1
0

The different answer between the two calls is what makes the test fail.

@pcanal
Copy link
Member

pcanal commented Apr 19, 2023

Another instance of the issue can be seen at: https://root-forum.cern.ch/t/possible-memory-leakage/54519

@pcanal
Copy link
Member

pcanal commented Apr 19, 2023

in #10454 (comment), the typedef is not being resolved, i.e.

enum EFruit {kApple=78, kBanana=29, kCitrus=34};

void repro()
{
    auto type = "std::vector<EFruit>::value_type";
    cout << gInterpreter->ClassInfo_IsEnum(type) << endl;
    cout << TEnum::GetEnum(type) << endl;
    auto resolved = TClassEdit::ResolveTypedef (type, kTRUE);
    cout << TEnum::GetEnum(resolved.c_str()) << endl;
}

prints

1
0
0x2048750

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

No branches or pull requests

6 participants