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

SCRAM has no knowledge on dictionary dependency #14473

Closed
davidlt opened this issue May 12, 2016 · 15 comments
Closed

SCRAM has no knowledge on dictionary dependency #14473

davidlt opened this issue May 12, 2016 · 15 comments

Comments

@davidlt
Copy link
Contributor

davidlt commented May 12, 2016

Fixing a bug (#14452) revealed another issue with the build tool, i.e. SCRAM.

In the PR edmWriteConfigs is segfaulting when loading libDataFormatsEgammaTrackReco.so dictionary. In the PR I changed DataFormats/GeometrySurface/interface/Surface.h and where are 144 packages which depend on DataFormats/GeometrySurface. The change breaks ABI by changing class layout due to multiple inheritance implementation in C++. Not all the packages have been rebuilt before libDataFormatsEgammaTrackReco.so was used by SCRAM.

ROOT (i.e. Cling) runs with auto load enabled. Cling new that missing piece in libDataFormatsEgammaTrackReco.so was provided by libTrackingToolsPatternTools.so due to rootmap files. So, it loaded it:

1281      23008:
1282      23008: file=/mnt/build/davidlt/new/another/CMSSW_8_1_X_2016-05-11-1100/lib/slc6_amd64_gcc530/libTrackingToolsPatternTools.so [0];  needed by /cvmfs/cms-ib.cern.ch/week0/slc6_amd64_gcc530/cms/cmssw-patch/CMSSW_8_1_X_2016-05-11-1100/external/slc6_amd64_gcc530/lib/libCling.so [0]
1283      23008: file=/mnt/build/davidlt/new/another/CMSSW_8_1_X_2016-05-11-1100/lib/slc6_amd64_gcc530/libTrackingToolsPatternTools.so [0];  generating link map
1284      23008:   dynamic: 0x00007fe45e98d1f8  base: 0x00007fe45e936000   size: 0x000000000005bc48
1285      23008:     entry: 0x00007fe45e9492d0  phdr: 0x00007fe45e936040  phnum:                  6

At that time libTrackingToolsPatternTools.so was not yet rebuilt with new ABI changes.

Here is what Cling actually needed from libTrackingToolsPatternTools.so:

export ROOTDEBUG=1
[..]
TClass::GetClass: HInfo in <TUnixSystem::Load>: loaded library /mnt/build/davidlt/new/another/CMSSW_8_1_X_2016-05-11-1100/lib/slc6_amd64_gcc530/libTrackingToolsPatternTools.so, status 0
Info in <TInterpreter::TCling::AutoLoad>: loaded library libTrackingToolsPatternTools.so for edm::Ref<vector<Trajectory>,Trajectory,edm::refhelper::FindUsingAdvance<vector<Trajectory>,Trajectory> >
@cmsbuild
Copy link
Contributor

cmsbuild commented May 12, 2016

A new Issue was created by @davidlt .

@davidlange6, @smuzaffar, @Degano, @davidlt, @Dr15Jones can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@Dr15Jones
Copy link
Contributor

Our policy has been that if a DataFormats library depends, even just through headers, another DataFmats package then the BuildFile.xml file should reflect that. That would also allow scram to know about the dependencies.

@davidlt
Copy link
Contributor Author

davidlt commented May 12, 2016

There is a idea for a fast checker script. I will explore that idea for the next week meeting.

@davidlt
Copy link
Contributor Author

davidlt commented May 12, 2016

assign core

@cmsbuild
Copy link
Contributor

New categories assigned: core

@Dr15Jones,@smuzaffar you have been requested to review this Pull request/Issue and eventually sign? Thanks

@davidlt
Copy link
Contributor Author

davidlt commented May 12, 2016

Combined output from a concept script: http://davidlt.web.cern.ch/davidlt/vault/combined_dict.log
It look <6 minutes to run it on lxplus 8 core machine for a full release (CMSSW_8_1_X_2016-05-11-2300)

~/testv3.sh $CMSSW_RELEASE_BASE/lib $PWD/ret  375.26s user 149.73s system 152% cpu 5:43.33 total
#!/bin/bash

# For example: $CMSSW_BASE/lib
TOPDIR=$1
RESULTDIR=$2

rm -rf $RESULTDIR
mkdir -p $RESULTDIR

# Create a temporary python checker
PY_CHECK=$(mktemp -t "$(hostname).$USER.XXXXXXXXXX.py")

cat << EOF > "$PY_CHECK"
#!/usr/bin/env python

from sys import exit, argv
import ROOT

ROOT.PyConfig.DisableRootLogon = True

ROOT.gROOT.SetBatch(True)
ROOT.gROOT.ProcessLine(".autodict")
ROOT.gROOT.ProcessLine("gDebug = 1;")
print(">>>>> Dictionary: {0}".format(argv[1]))
if ROOT.gSystem.Load(argv[1]) < 0:
    print(">>>>> Error: Cannot load the dictionary!")
    exit(1)

rootmap = {}

with open(argv[2], "r") as rootmap_file:
    ignore = True
    for line in rootmap_file:
        line = line.strip()
        # Ignore comments
        if line.startswith("#"):
            continue
        # Ignore until records
        if line == "[ {0} ]".format(argv[1]):
            ignore = False
            continue
        if ignore == True:
            continue
        key, val = line.split(" ", 1);
        if key not in rootmap:
            rootmap[key] = []
        rootmap[key].append(val)

for cl in rootmap["class"]:
    print(">>>>> Class {0}".format(cl))
    c = ROOT.TClass.GetClass(cl)
    if not c:
        print(">>>>> Error: Failed to load the class")

for cl in rootmap["enum"]:
    print(">>>>> Enum {0}".format(cl))
    c = ROOT.TClass.GetClass(cl)
    if not c:
        print(">>>>> Error: Failed to load the enum")

exit(0)
EOF

chmod +x "$PY_CHECK"

DICT_ALL=""
for ROOTMAP in $(find "$TOPDIR" -name '*.rootmap' -type f); do
    DICT=$(grep -o '^\[ .* \]$' "$ROOTMAP" | cut -d' ' -f 2)
    if [ ! -z "$DICT" ]; then
        DICT_ALL="$DICT_ALL $DICT $ROOTMAP"
    fi
done

#echo "$DICT_ALL" | sed 's/^ *//g;s/ lib/\nlib/g'
echo "$DICT_ALL" | sed 's/^ *//g;s/ lib/\nlib/g' | xargs -P $(getconf _NPROCESSORS_ONLN) -t -I%  sh -c "$PY_CHECK % 2>&1 | grep -E '>>>>>|TCling::AutoLoad' > $RESULTDIR/\$RANDOM.log 2>&1"

rm -f "$PY_CHECK"

There are additional interesting things, e.g. root map file without a dictionary (maybe different name?) and empty root map files (empty dictionaries?) We found one empty dictionary with Conditions with @smuzaffar today, but maybe there are more.

@davidlt
Copy link
Contributor Author

davidlt commented May 12, 2016

TInterpreter::TCling::AutoLoad was triggered 222 times. Once you add a link dependency via BuildFile.xml then TInterpreter::TCling::AutoLoad is gone (only tried once).

@davidlt
Copy link
Contributor Author

davidlt commented May 12, 2016

Example.

>>>>> Dictionary: libDataFormatsTrajectorySeed.so
>>>>> Class TrajectorySeed
Info in <TInterpreter::TCling::AutoLoad>: loaded library libDataFormatsTrajectoryState.so for PTrajectoryStateOnDet
[..]

Then:

Looks like DataFormats/TrajectorySeed in reality depends on DataFormats/TrajectoryState.

@davidlt
Copy link
Contributor Author

davidlt commented May 12, 2016

/cc @slava77

@smuzaffar
Copy link
Contributor

@davidlt, great work.
we should make it part of release and PR tests

@davidlt
Copy link
Contributor Author

davidlt commented May 12, 2016

We can write a better tools (this was a concept). It runs very fast thus it could be added to PR tests.

@davidlt
Copy link
Contributor Author

davidlt commented May 16, 2017

CC @mrodozov

@smuzaffar
Copy link
Contributor

assign externals

@cmsbuild
Copy link
Contributor

New categories assigned: externals

@smuzaffar,@gudrutis,@mrodozov you have been requested to review this Pull request/Issue and eventually sign? Thanks

@smuzaffar
Copy link
Contributor

no going to implement this, this will be fixed with CXXMODULES enabled

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

4 participants