Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-8728

SCons pybind11 builds re-link unnecessarily

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      When rebuilding the pybind11 branch of afw, I noticed that scons was frequently rebuilding products even when no changes had occurred. Knowing that this used to be a problem in Swig builds based on whether SCons processed Swig files before or after .cc files, I tried explicitly listing the build targets, as this was the workaround for the Swig problem. This did prevent an immediate build, but revealed another way to trigger the problem that may be easier to debug:

      If you do:

      scons lib python

      and then

      scons lib python tests

      You'll note that it re-links all of the pybind11 modules before running all of the tests. If you run scons with --debug=explain, it will report that the dependencies of libafw.so have changed, and hence that file needs to be re-linked. It then rebuilds all of the pybind11 modules, since these each link against libafw.so. I don't understand why it thinks the dependencies of libafw.so change when we add tests to the target list (they may also change under other weird circumstances), but that's what we should fix.

      Giving this a lot of SPs since SCons dependency logic debugging can be hard.

        Attachments

          Issue Links

          blocks

          Story - Created by Jira Software - do not edit or delete. Issue type for a user story. DM-9100 Merge pybind11 branches to master

          • Done

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            jbosch Jim Bosch added a comment -

            Fun discoveries, which seem like they have to be SCons bugs of some kind:

            • libafw.so is rebuilt if you add or remove "tests" from the list of targets, because doing so changes the order of its dependencies (a list of .os files). This happens despite the fact that immediately before we call SharedLibrary with a list of sources (.cc files), we sort that list. We might be able to work around this by manually generating the list of .os files with SharedObject and then sorting those and passing them to SharedLibrary, but that may just be wishful thinking.
            • When libafw.so is rebuilt, all of the pybind11 modules that link to it are re-linked, because SCons claims that it has changed. However, we've told SCons to use md5 sums to decide when something has changed, and the md5 sum of libafw.so hasn't changed. I haven't yet been able to find any way to ask SCons whether it understands that it's supposed to use md5 sums, or found out where it's saving those hashes.
            Show
            jbosch Jim Bosch added a comment - Fun discoveries, which seem like they have to be SCons bugs of some kind: libafw.so is rebuilt if you add or remove "tests" from the list of targets, because doing so changes the order of its dependencies (a list of .os files). This happens despite the fact that immediately before we call SharedLibrary with a list of sources (.cc files), we sort that list. We might be able to work around this by manually generating the list of .os files with SharedObject and then sorting those and passing them to SharedLibrary , but that may just be wishful thinking. When libafw.so is rebuilt, all of the pybind11 modules that link to it are re-linked, because SCons claims that it has changed. However, we've told SCons to use md5 sums to decide when something has changed, and the md5 sum of libafw.so hasn't changed. I haven't yet been able to find any way to ask SCons whether it understands that it's supposed to use md5 sums, or found out where it's saving those hashes.
            Hide
            Permalink
            jbosch Jim Bosch added a comment -

            Not promising I'm going to be the one to finish this, but I'm definitely working on it (mostly waiting for test runs to finish building).

            Show
            jbosch Jim Bosch added a comment - Not promising I'm going to be the one to finish this, but I'm definitely working on it (mostly waiting for test runs to finish building).
            Hide
            Permalink
            jbosch Jim Bosch added a comment -

            I believe this is ready for review. Tim Jenness, could you take a look?

            I've only fixed one of the problems I noted above - the fact that the lib target spuriously rebuilt when the top-level build targets change. That was due to dependency ordering changes, which was in turn due to sorting of those dependencing not working. With that change, the second problem seems much harder to trigger (possibly impossible to trigger accidentally), so I'm much less concerned about it. I'd love to fix that too, but I have no idea how to debug it, let alone fix it.

            Note that while this is part of the pybind11 epic since it's a much more obvious problem on the pybind11 branches, the problem is not limited to the pybind11 branch and hence I've made my changes directly on master and intend to merge to master when complete; we can then rebase the pybind11 branches on that.

            In addition to the changes in sconsUtils, I've also replaced the afw/lib/SConscript file with the standard boilerplate; it seems to have just been reimplementation of the relevant part of BasicSConscript.lib(), rather than a meaningful difference. GitHub PRs will be up shortly, and Jenkins is in progress.

            Show
            jbosch Jim Bosch added a comment - I believe this is ready for review. Tim Jenness , could you take a look? I've only fixed one of the problems I noted above - the fact that the lib target spuriously rebuilt when the top-level build targets change. That was due to dependency ordering changes, which was in turn due to sorting of those dependencing not working. With that change, the second problem seems much harder to trigger (possibly impossible to trigger accidentally), so I'm much less concerned about it. I'd love to fix that too, but I have no idea how to debug it, let alone fix it. Note that while this is part of the pybind11 epic since it's a much more obvious problem on the pybind11 branches, the problem is not limited to the pybind11 branch and hence I've made my changes directly on master and intend to merge to master when complete; we can then rebase the pybind11 branches on that. In addition to the changes in sconsUtils, I've also replaced the afw/lib/SConscript file with the standard boilerplate; it seems to have just been reimplementation of the relevant part of BasicSConscript.lib() , rather than a meaningful difference. GitHub PRs will be up shortly, and Jenkins is in progress.
            Hide
            Permalink
            tjenness Tim Jenness added a comment -

            Looks okay to me.

            Show
            tjenness Tim Jenness added a comment - Looks okay to me.
            Hide
            Permalink
            jbosch Jim Bosch added a comment -

            Merged to master.

            Show
            jbosch Jim Bosch added a comment - Merged to master.

              People

              Assignee:
              jbosch Jim Bosch
              Reporter:
              jbosch Jim Bosch
              Reviewers:
              Tim Jenness
              Watchers:
              Jim Bosch, Pim Schellart [X] (Inactive), Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: