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

cpp_info components #5408

Closed
wants to merge 149 commits into from
Closed

cpp_info components #5408

wants to merge 149 commits into from

Conversation

danimtb
Copy link
Member

@danimtb danimtb commented Jun 27, 2019

Changelog: Feature: Extended cpp_info model to support different components inside the same package such as different libraries, executables...
Docs: conan-io/docs#1363

@PYVERS: py34
@tags: slow

Closes #5090
Closes #5242
Closes #2382
Closes #2387

value = value[0]
if key in ["includedirs", "libdirs", "bindirs", "builddirs", "srcdirs",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure why this is being changed, and why this processing is necessary now. Any idea @lasote?

Copy link
Contributor

@lasote lasote Jul 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say previously you were getting the aggregated info and now it is getting the components info to aggregate it later.

@memsharded
Copy link
Member

I finally managed to run some performance tests. This branch, merged with develop, against develop.

The resolution of a fully cached and built graph of 200 packages increases with this change from 6.2 seconds to 9.9 seconds. A 100 packages graph from 2.1 to 3.1 seconds. So seems it is 50% added time, linear.

@lasote lasote modified the milestones: 1.18, 1.19 Jul 29, 2019
@lasote
Copy link
Contributor

lasote commented Jul 29, 2019

That is not acceptable. We need to check where the time is going. Assigned to 1.19 with look into.

@memsharded
Copy link
Member

I have been running some extra tests, to see where the time is going. The reason is the DepCppInfo() massive object creation.

When packages binaries are installed, their information is propagated downstream with:

    def _propagate_info(node):
        # Get deps_cpp_info from upstream nodes
        node_order = [n for n in node.public_closure if n.binary != BINARY_SKIP]
        # List sort is stable, will keep the original order of the closure, but prioritize levels
        conan_file = node.conanfile
        for n in node_order:
            ...
            conan_file.deps_cpp_info.update(n.conanfile.cpp_info, n.ref.name)

This is done per node in the graph.

This PR code does:

    def update(self, cpp_info, pkg_name):
        assert isinstance(cpp_info, CppInfo)
        self.update_dep_cpp_info(DepCppInfo(cpp_info), pkg_name)

That is, it creates a new DepCppInfo every time. This is repeated many times even for the same package. As every DepCppInfo node pre-computes and caches with:

        self._bindirs = self._get_relative_dirs("bin")
        self._builddirs = self._get_relative_dirs("build")
        self._include_paths = self._get_absolute_paths("include")
        self._src_paths = self._get_absolute_paths("src")
        self._lib_paths = self._get_absolute_paths("lib")

And those are relatively heavy operations, with several function calls, conditionals, etc, the overall impact is huge.

It is irrelevant where this computation is done here or later (lazy), the impact would be just delayed, because they are copies, with the exact intention to protect the original object via this layer.

@memsharded
Copy link
Member

This has been superseded by more recent work on components, some already merged, some ongoing.

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