Use refcount based gc (shared_ptr) when Boehm GC is disabled#5640
Draft
ChrisDodd wants to merge 16 commits into
Draft
Use refcount based gc (shared_ptr) when Boehm GC is disabled#5640ChrisDodd wants to merge 16 commits into
ChrisDodd wants to merge 16 commits into
Conversation
fruffy
added
bmv2
Collaborator
|
This is great!! One way to test is to disable GC for CI and check what happens. We can also disable for testgen in general, which has long running tests sensitive to leaking memory. Curious about the performance of this too. |
ChrisDodd
force-pushed
the
cdodd-disablegc
branch
from
386547a to
f410234
Compare
- IR::Ptr that maps for 'const *' or IR::shared_ptr depending on HAVE_LIBGC Signed-off-by: Chris Dodd <cdodd@nvidia.com>
Signed-off-by: Chris Dodd <cdodd@nvidia.com>
- fields held in P4::MethodInstance objects and other subclasses - return values of typechecking canonicalize and specialize methods - returned value in bindVariables - CloneExpressions::clone return value - FrentEnd::run return value - typeMap::setType argument, due to strangess with absl maps apparently destroying the fowarded argument before copying it. - check for unreferenced raw pointers in typeMap that would otherwise get deleted, due to its internal use of IR::Ptr Also fix IR ctors to not use delegating ctors, as they evaluate the delegated args *before* calling the base class ctor, so screws up IR::shared_ptr stuff figuring out if an object is on the heap or not - apply_visitor routines need to ensure there's a ref to the node to be returned *before* calling visited.reset() Signed-off-by: Chris Dodd <cdodd@nvidia.com>
- preorder/postorder methods all return raw pointers to allow using covariant return typing to return something more specific than an IR::Node pointer -- only works for raw pointers, not smart pointers - guardReturn stashes a copy of an IR::Ptr temporarily in the Transform so it won't be freed when returning as a raw pointer if there are no other references Signed-off-by: Chris Dodd <cdodd@nvidia.com>
- ProgramMap peristent handles to P4Program - temps in hsIndexSimplify - ReferenceMap internal caches - Inlining held values - ConstantTypeSubstitution convert method return values - TypeConstraint internals/caches Can't safely return a pointer to an inline field, as the containing object might be freed; need explicit clone in DoSimplifyControlFlow Signed-off-by: Chris Dodd <cdodd@nvidia.com>
- constantFolding internal cloning - DoReplaceTuples - DoLocalCopyPropagation - specializeGenericTypes/ReplaceTypeUses - irutils.cpp - eliminateTypedefs/DoReplaceTypedef - defaultArguments/TypeNameSubstitutionVisitor Signed-off-by: Chris Dodd <cdodd@nvidia.com>
- inlining/GeneralInline temp - removeParameters/DoRemoveActionParameters temps - sideEffects/DoSimplifyExpressions created temp vars and additions - specialization (SpecializeFunctions/TypeSpecialization) added things - getP4Type() return value cached many places - typeChecker/TypeInferenceBase::constantFold return value - more typechecking temp/cached values - TypeSubstitution bindings - TypeMap leftValues and constants being inserted into maps - midend flattening transforms temps - midend/interperter symbolic values - parserUnroll/ParserSymbolicInterpreter created temps - unrollLoops temp body being constructed Inspector Visitor::Tracker remove node from cache if it is to be revisted. Reduces size of the tracer cache for visitDagOnce = false visitors and avoid dangling temp problem with toP4 Signed-off-by: Chris Dodd <cdodd@nvidia.com>
- move const so we can have both `IR::Ptr` and `IR::MutablePtr` Signed-off-by: Chris Dodd <cdodd@nvidia.com>
- bmv2/dpdk/ebpf/udbpf changes to build and (mostly?) work - tofino inital chages to main/frontend - minor typecheck fixes - Evaluator fixes using IR::MutablePtr and IR::Ptr Signed-off-by: Chris Dodd <cdodd@nvidia.com>
Signed-off-by: Chris Dodd <cdodd@nvidia.com>
Signed-off-by: Chris Dodd <cdodd@nvidia.com>
Signed-off-by: Chris Dodd <cdodd@nvidia.com>
Signed-off-by: Chris Dodd <cdodd@nvidia.com>
Signed-off-by: Chris Dodd <cdodd@nvidia.com>
Signed-off-by: Chris Dodd <cdodd@nvidia.com>
- do NOT use IR::Ptr for the return value of getWriteDest in parde.def, so that we can have covariant return types. - stateful alu creation use IR::Ptr throughout - fromv1.0 stuff use IR::Ptr is most places - MANY changes 'const auto *' => 'auto' to allow inferring IR::Ptr Signed-off-by: Chris Dodd <cdodd@nvidia.com>
ChrisDodd
force-pushed
the
cdodd-disablegc
branch
from
f410234 to
05542be
Compare
fruffy
added
the
p4tc
fruffy
added
run-validation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a huge change that touches a lot of code but doesn't really do anything.
The main change is to replace
const IR::NodeType *in 1000s of places withIR::Ptr<NodeType>, and when the compiler is configured theENABLE_GC=OFFuse a reference counted 'smart' pointer for that. In the normal build case,IR::Ptr<T>is just an alias forconst T *, so there's no real change.We use a custom
IR::shared_ptr(rather thanstd::shared_ptr), as we use IR::Node objects allocated on the stack or embedded in other objects (withinlinein .def files) in many cases, so we need to detect when objects are allocated withnewand when they are not, and only use the refcount to clean up those allocated withnewThere are still many places in the compiler that will leak memory (for things that are not IR nodes); those will need to be individually changed to use std::shared_ptr (or equivalent) to avoid leaking.
Note that this PR does not (yet) contain fixes for the tofino backend to make it build/work with ENABLE_GC=OFF