Map v3

[klingaard]

Been chatting about making a v3 of MAP that addresses lots of annoyances/issues/missing features of the framework.

Enhancements

• Remove the macros CREATE_ and use real C++ methodologies to obtain the callbacks
• All Resources should adhere to the common container traits of modern C++. For example, see issue Add missing methods to Buffer and Queue  #247
• SimDB will be revamped if not decoupled. @cnyce is focusing on this now.
  -- Related issues: sparta ReportHeader is clumsy with stdlib types #44, Disable pipeline collection on strings #13, Conditionally compile Collectable::collect on ostream operator availability  #45, Remove pipeline collection functionalities from Resources #55
• Better DAG cyclic debugging. The current methodology is to generate a .dot file with the cycle. This is useful, but ... not. Also, the DAG includes in the cyclic error, nodes that are really part of the cyclic behavior like the sparta phase groups.
• Eliminate the need for the resource factory class to outlive the actual resource. The ResourceTreeNode uses the factory to tear down the resource, which means the resource and the factor are always tied at the hip. Think of way to eliminate that requirement.

Cleanup

• The internal resources (Buffer, Queue, Array) should look/feel like C++ STL types. I.e. erase should return the next iterator, etc
• sparta::TreeNode was cleaned up a bit, but can use more cleanup to reduce compilation time
• Run clang-format
• Sparta generated messages to cout or cerr should be prepended with something like SPARTA: so developers know where the message originated

Bug Fixes

Reports

• When using notifications, you cannot use boolean types: 'true' or 'false'
• If emitted type of the notification does not match the type of expected value of the notification in a report, the error message is NOT helpful. The following example expects an unsigned long, but if the notification is an unsigned int, the error message just says, "cannot find the notification"

    trigger:
      whenever: notif.rob_start_inst == 1
  

• Be great to create multiple reports for every trigger hit. In other words, create report1.out when the trigger is hit the first time, report2.out when hit the second time, etc

[bdutro]

Enhancements

• Find an alternative to Conda for managing the development environment
  • Reasoning:
    • Conda's toolchain support is on a best-effort basis
    • Conda injects suboptimal CFLAGS into the environment
    • Conda generally doesn't package static libraries
    • Version pinning can be flaky and usually requires additional tooling (see Prefix-dev below)
  • Potential replacements:
    • Docker
      • Simple (possibly simplest?) option for local development
      • Can be a problem for internal IT deployments (requires additional user permissions to spin up/down containers, daemon runs as root, etc.)
      • Not a true shared environment
        • Individual users will be in their own container instances
        • Requires binding work directories from the host system into the container
        • Simulator will need to be statically linked in order to run outside of the container environment
    • Podman, Singularity
      • Alternative Docker-like container solutions
      • Similar downsides to Docker, however both of these solutions can be run without a daemon and without root privileges
    • Spack
      • More Conda-like (doesn't use containers, works by modifying the environment of a session)
      • Majority of packages are built from source - both a pro and a con since the packages will Just Work, but some packages can take forever to build (LLVM)
      • There is a central Spack mirror for prebuilt packages, but environments must match exactly for them to work
      • Can set up internal prebuilt package repo (build once, install env many times)
        • Note that this can also be done with Conda
    • Nix
      • Can be installed on non-NixOS systems to provide Nix packages
      • nix-user-chroot gives a rootless chroot into a Nix system without affecting the underlying OS installation
        • This proved to be surprisingly brittle when I tried to get it working, but someone more familiar with Nix may have better luck
    • Prefix-dev
      • Basically provides lockfiles for Conda
      • Inherits the other problems listed above under Reasoning
    • vcpkg
      • C++ library package manager
      • Doesn't provide toolchains
    • Conan
      • Another C++ library package manager like vcpkg
      • Doesn't provide toolchains
    • xmake
      • Combined build/run/debug system with a library dependency package manager
      • Also provides ability to install toolchains
      • I'm not sure if it can be used to create a centrally available development environment like Conda
        • Appears to be more like cargo/npm/virtualenv/etc. where the build environment is expected to live alongside the project files
      • Requires an additional layer of build configuration

[timsnyder]

I'm not opposed to considering something other than conda for providing dependencies. I know there have been breakages lately that I haven't had time to look into. Conda definitely has its warts but I haven't seen anything that enables managing an environment for our internal batch farm. I also don't personally care about local development because we aren't able to develop locally where I am.

Conda injects suboptimal CFLAGS into the environment

The method of using conda that I helped add into FireSim/Chipyard doesn't end up with the conda CFLAGS in the environment and we could move to that. Of course, all of the packages we depend on will be built with the CFLAGS. I have seen folks in the conda-forge toolchain groups talking about moving away from specifying all of those flags but it has been awhile since I actively looked at what the latest toolchains inject.

Conda generally doesn't package static libraries

When I looked earlier this year, I was able to find static versions of everything (the older version of Sparta we use) needed. Which things are missing?

Prefix-dev

This looks interesting (I haven't seen it before). I know FireSim/Chipyard used conda-lock with conda to be able to use a lockfile to decouple environment solving from installing all of the packages needed.

Inherits many of the other problems of Conda's packaging philosophy

It would be useful to enumerate those problems or clarify whether you already have (above under Reasoning).

Docker/Podman

Even using podman, at my current place, we're unable to internally use containers on our large compute farm. IT is slowly moving forward with allowing use but there is not a general solution where one can use containers ubiquitously.

[bdutro]

Conda generally doesn't package static libraries

When I looked earlier this year, I was able to find static versions of everything (the older version of Sparta we use) needed. Which things are missing?

Generally speaking, any packages from conda-forge may or may not have static libraries.

A couple examples of this that affect Sparta:

• Boost: Missing static libraries? conda-forge/boost-feedstock#159
• HDF5: Getting rid of the -static builds conda-forge/hdf5-feedstock#185

It might be possible to avoid those problems if the static libraries are packaged in default, but anyone using default in a large company could run into licensing issues there. Also, it's usually just a matter of time before someone needs a package that's only available in conda-forge, and that may require updating a default package with a conda-forge equivalent that doesn't include static libraries.

I suppose we could sidestep this entirely by maintaining a sparcians conda channel with our own builds of the Sparta library dependencies that include static libraries.

It would be useful to enumerate those problems or clarify whether you already have (above under Reasoning).

Yes, that's what I meant. Updated to clarify that.

[timsnyder]

Sorry for the delayed reply. I've been sick recently but I'm back.

Ugh. Static Libraries. Yeah, you're right...

Good point about Boost and HDF5. I forgot that my looking earlier this year is still on older versions of Sparta and updating is another thing on the list to do at some point. When conda-forge cfep-18 setting a policy for static libraries, each package has dropped them and then required some work to add back a separate package. In general, I haven't encountered any resistance to adding the split package back, especially when I help create the PR to do it. I started the convo for HDF5 in conda-forge/hdf5-feedstock#185. I'll try to prioritize getting them re-added.

Stepping back a bit, I know why I need static libraries and it has to do with weird internal usecase where I want to ship a single .so with almost everything (except libc) bundled into it to another company so that I don't have to deal with them needing to know how to install conda. But, in general, does Sparta need to have the static libs?

Yes, I'd stay away from default

It might be possible to avoid those problems if the static libraries are packaged in default

Yeah, as you mention, I would avoid using the Anaconda channels entirely because of the licensing issues you link to. I would suggest only using conda-forge or other channels that do not encumber the packages like Anaconda has done.

Having a sparcians channel...

I suppose we could sidestep this entirely by maintaining a sparcians conda channel with our own builds of the Sparta library dependencies that include static libraries.

It is probably less effort initially and definitely over time to maintain if we contribute to the conda-forge recipes.

However, what if we to create CI builds to build custom sparcians container images that have the conda packages installed. This could allow for container-based and container-deprived users to still use the same set of tools and also have a more robust mechansim (than either Prefix-dev or conda-lock) to further decouple container-based users (including our own CI) from the periodic breakages or outages that happen from time to time with the conda ecosystem. I've seen messages about sparcians CI being intermittently broken recently but again, not enough time to ask if help was needed.

The container images could be hosted in a sparcians channel of whatever registry is free for open-source use.

[timsnyder]

Better DAG cyclic debugging. The current methodology is to generate a .dot file with the cycle. This is useful, but ... not. Also, the DAG includes in the cyclic error nodes that are really part of the cyclic behavior.

@klingaard is there a typo in that somewhere? Were you meaning to say "Also, the DAG includes nodes in the error message that are not really a part of the cyclic behavior."? Can you clarify that point in your OP?

[klingaard]

Missed a comma. 😆 Updated it a bit. If it's still confusing, I'm ok with your updating it.

[timsnyder]

Remove the macros CREATE_ and use real C++ methodologies to obtain the callbacks

Does this include the PARAMETER macros? Well, I guess you say callbacks so that would imply handlers here but it would be nice if we could use C++ to generate the PARAMETERs as well.

[timsnyder]

Also for something like this are you going to have a deprecation path or just expect people to suck it up and migrate all their code en masse?

What I mean is add the C++'isms in a 2.X release but remove them in 3.0 so that they both exist in at least one release?

[klingaard]

Great questions.

Haven't put any thoughts into revamping the parameter interfaces; I'm opened to ideas/complaints/enhancements.

As far as rolling out changes...

I was thinking about staging the releases, with each release containing a major change.

For example, the first release, 3.0 might contain the separation of SimDB. 3.1 might remove the CREATE_SPARTA_HANDLER... macros along with a script to convert existing simulators to 3.1, etc.

[VictorYing]

Per discussion in #547: we should probably fix undefined behavior of destruction order of TreeNodes contained in the std::vector<std::unique_ptr<sparta::TreeNode>> to_delete_; member inside of the sparta::app::Simulation base class: Currently MAP displays opposite behavior on LLVM libc++ as it displays on GNU libstdc++

[klingaard]

Yeah, I saw the issue of vector deletion order a while ago between LLVM and GNU. It's annoying that this isn't a standard, but I also understand why.

Anyway, the to_delete_ in sparta::app::Simulation is there for pure convenience. Feel free to not use it (we don't) and maintain your own TreeNode memory management in your local instance of sparta::app::Simulation.

But with that said, the fact that the factories dellocation order is coupled with the TreeNode deletion order is more of an annoyance. IIRC, when Nick Haw and I chatted about this, we thought about having the factories keep track of the allocated objects for you, but that broke one of our requirements: "Sparta will only delete memory that Sparta explicitly allocates"

For map_v3 I want to simply remove to_delete_ from sparta::app::Simulation and force the user to manage their own memory thus alleviating the confusing and frustration that you're encountering. I don't think it's worth putting in fancy deallocation callbacks/policies.

But a few statements regarding your use case...

Having a TreeNode rely on the destruction order of other TreeNodes seems dangerous. In other words, having an object's destructor make use of a non-smart pointer to another allocated object not owned by the caller is asking for trouble.

If these are parent nodes that are traversing children during destruction, the parent should explicitly own those children or have the parent's factory derivative own them (the factory can create the parent, then the children, then ensure the deletion order).

[VictorYing]

I would not be opposed to removing to_delete_ entirely.

[klingaard]

Cool, let's do that. 😉