From 31223ca180b0b3199fa0c6a04e47548ec356150d Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Wed, 8 Feb 2017 14:34:39 +0100 Subject: [PATCH] - remove all symbols that get linked into the symbol table from the garbage collector. Symbols are very easy to manage once they are in a symbol table and there's lots of them so this reduces the amount of work the GC needs to do quite considerably. After cleaning out compile-time-only symbols there will still be more than 2000 left, one for each function and one for each member variable of a class or struct. This means more than 2000 object that won't need to tracked constantly by the garbage collector. Note that loose fields which do occur during code generation will be GC'd just as before. --- src/dobject.cpp | 70 ++++++++++++++++++------------- src/dobject.h | 3 ++ src/dobjgc.cpp | 6 ++- src/dobjtype.cpp | 26 +----------- src/dobjtype.h | 4 -- src/scripting/backend/codegen.cpp | 13 ------ src/scripting/backend/codegen.h | 1 - src/scripting/symbols.cpp | 52 +++++++---------------- src/scripting/symbols.h | 5 +-- 9 files changed, 64 insertions(+), 116 deletions(-) diff --git a/src/dobject.cpp b/src/dobject.cpp index 63c7bb688..68f3aab8e 100644 --- a/src/dobject.cpp +++ b/src/dobject.cpp @@ -301,42 +301,17 @@ DObject::~DObject () PClass *type = GetClass(); if (!(ObjectFlags & OF_Cleanup) && !PClass::bShutdown) { - DObject **probe; - - if (!(ObjectFlags & OF_YesReallyDelete)) + if (!(ObjectFlags & (OF_YesReallyDelete|OF_Released))) { Printf("Warning: '%s' is freed outside the GC process.\n", type != NULL ? type->TypeName.GetChars() : "==some object=="); } - // Find all pointers that reference this object and NULL them. - StaticPointerSubstitution(this, NULL); - - // Now unlink this object from the GC list. - for (probe = &GC::Root; *probe != NULL; probe = &((*probe)->ObjNext)) + if (!(ObjectFlags & OF_Released)) { - if (*probe == this) - { - *probe = ObjNext; - if (&ObjNext == GC::SweepPos) - { - GC::SweepPos = probe; - } - break; - } - } - - // If it's gray, also unlink it from the gray list. - if (this->IsGray()) - { - for (probe = &GC::Gray; *probe != NULL; probe = &((*probe)->GCNext)) - { - if (*probe == this) - { - *probe = GCNext; - break; - } - } + // Find all pointers that reference this object and NULL them. + StaticPointerSubstitution(this, NULL); + Release(); } } @@ -347,6 +322,41 @@ DObject::~DObject () } } +void DObject::Release() +{ + DObject **probe; + + // Unlink this object from the GC list. + for (probe = &GC::Root; *probe != NULL; probe = &((*probe)->ObjNext)) + { + if (*probe == this) + { + *probe = ObjNext; + if (&ObjNext == GC::SweepPos) + { + GC::SweepPos = probe; + } + break; + } + } + + // If it's gray, also unlink it from the gray list. + if (this->IsGray()) + { + for (probe = &GC::Gray; *probe != NULL; probe = &((*probe)->GCNext)) + { + if (*probe == this) + { + *probe = GCNext; + break; + } + } + } + ObjNext = nullptr; + GCNext = nullptr; + ObjectFlags |= OF_Released; +} + //========================================================================== // // diff --git a/src/dobject.h b/src/dobject.h index 7c2fd4c22..82051e3bd 100644 --- a/src/dobject.h +++ b/src/dobject.h @@ -468,6 +468,9 @@ public: Class = NULL; } + // Releases the object from the GC, letting the caller care of any maintenance. + void Release(); + // For catching Serialize functions in derived classes // that don't call their base class. void CheckIfSerialized () const; diff --git a/src/dobjgc.cpp b/src/dobjgc.cpp index ca57e6aca..e6ff9c734 100644 --- a/src/dobjgc.cpp +++ b/src/dobjgc.cpp @@ -277,7 +277,9 @@ static DObject **SweepList(DObject **p, size_t count, size_t *finalize_count) void Mark(DObject **obj) { DObject *lobj = *obj; - if (lobj != NULL) + + assert(lobj == nullptr || !(lobj->ObjectFlags & OF_Released)); + if (lobj != nullptr && !(lobj->ObjectFlags & OF_Released)) { if (lobj->ObjectFlags & OF_EuthanizeMe) { @@ -551,6 +553,8 @@ void Barrier(DObject *pointing, DObject *pointed) assert(pointing == NULL || (pointing->IsBlack() && !pointing->IsDead())); assert(pointed->IsWhite() && !pointed->IsDead()); assert(State != GCS_Finalize && State != GCS_Pause); + assert(!(pointed->ObjectFlags & OF_Released)); // if a released object gets here, something must be wrong. + if (pointed->ObjectFlags & OF_Released) return; // don't do anything with non-GC'd objects. // The invariant only needs to be maintained in the propagate state. if (State == GCS_Propagate) { diff --git a/src/dobjtype.cpp b/src/dobjtype.cpp index 3c556da3b..f0e7b2fc1 100644 --- a/src/dobjtype.cpp +++ b/src/dobjtype.cpp @@ -185,18 +185,6 @@ PType::~PType() { } -//========================================================================== -// -// PType :: PropagateMark -// -//========================================================================== - -size_t PType::PropagateMark() -{ - size_t marked = Symbols.MarkSymbols(); - return marked + Super::PropagateMark(); -} - //========================================================================== // // PType :: WriteValue @@ -2413,7 +2401,7 @@ PField *PStruct::AddField(FName name, PType *type, DWORD flags) if (Symbols.AddSymbol(field) == nullptr) { // name is already in use - delete field; + field->Destroy(); return nullptr; } Fields.Push(field); @@ -2444,18 +2432,6 @@ PField *PStruct::AddNativeField(FName name, PType *type, size_t address, DWORD f return field; } -//========================================================================== -// -// PStruct :: PropagateMark -// -//========================================================================== - -size_t PStruct::PropagateMark() -{ - GC::MarkArray(Fields); - return Fields.Size() * sizeof(void*) + Super::PropagateMark(); -} - //========================================================================== // // NewStruct diff --git a/src/dobjtype.h b/src/dobjtype.h index cfae5e169..6c07b047b 100644 --- a/src/dobjtype.h +++ b/src/dobjtype.h @@ -171,8 +171,6 @@ public: const char *DescriptiveName() const; - size_t PropagateMark(); - static void StaticInit(); }; @@ -520,8 +518,6 @@ public: virtual PField *AddField(FName name, PType *type, DWORD flags=0); virtual PField *AddNativeField(FName name, PType *type, size_t address, DWORD flags = 0, int bitvalue = 0); - size_t PropagateMark(); - void WriteValue(FSerializer &ar, const char *key,const void *addr) const override; bool ReadValue(FSerializer &ar, const char *key,void *addr) const override; void SetDefaultValue(void *base, unsigned offset, TArray *specials) const override; diff --git a/src/scripting/backend/codegen.cpp b/src/scripting/backend/codegen.cpp index a3dc066e9..910dba19f 100644 --- a/src/scripting/backend/codegen.cpp +++ b/src/scripting/backend/codegen.cpp @@ -6558,23 +6558,10 @@ FxStackVariable::FxStackVariable(PType *type, int offset, const FScriptPosition FxStackVariable::~FxStackVariable() { - // Q: Is this good or bad? Needs testing if this is fine or better left to the GC anyway. DObject's destructor is anything but cheap. membervar->ObjectFlags |= OF_YesReallyDelete; delete membervar; } -//========================================================================== -// -// -//========================================================================== - -void FxStackVariable::ReplaceField(PField *newfield) -{ - membervar->ObjectFlags |= OF_YesReallyDelete; - delete membervar; - membervar = newfield; -} - //========================================================================== // // diff --git a/src/scripting/backend/codegen.h b/src/scripting/backend/codegen.h index c64ec2094..abbcbd54e 100644 --- a/src/scripting/backend/codegen.h +++ b/src/scripting/backend/codegen.h @@ -1417,7 +1417,6 @@ class FxStackVariable : public FxMemberBase public: FxStackVariable(PType *type, int offset, const FScriptPosition&); ~FxStackVariable(); - void ReplaceField(PField *newfield); FxExpression *Resolve(FCompileContext&); bool RequestAddress(FCompileContext &ctx, bool *writable); ExpEmit Emit(VMFunctionBuilder *build); diff --git a/src/scripting/symbols.cpp b/src/scripting/symbols.cpp index 68b9d522c..b6adb33e7 100644 --- a/src/scripting/symbols.cpp +++ b/src/scripting/symbols.cpp @@ -149,34 +149,19 @@ PSymbolTable::~PSymbolTable () //========================================================================== // -// -// -//========================================================================== - -size_t PSymbolTable::MarkSymbols() -{ - size_t count = 0; - MapType::Iterator it(Symbols); - MapType::Pair *pair; - - while (it.NextPair(pair)) - { - GC::Mark(pair->Value); - count++; - } - return count * sizeof(*pair); -} - -//========================================================================== -// -// +// this must explicitly delete all content because the symbols have +// been released from the GC. // //========================================================================== void PSymbolTable::ReleaseSymbols() { - // The GC will take care of deleting the symbols. We just need to - // clear our references to them. + auto it = GetIterator(); + MapType::Pair *pair; + while (it.NextPair(pair)) + { + delete pair->Value; + } Symbols.Clear(); } @@ -243,6 +228,7 @@ PSymbol *PSymbolTable::AddSymbol (PSymbol *sym) return nullptr; } Symbols.Insert(sym->SymbolName, sym); + sym->Release(); // no more GC, please! return sym; } @@ -257,6 +243,7 @@ void PSymbolTable::RemoveSymbol(PSymbol *sym) auto mysym = Symbols.CheckKey(sym->SymbolName); if (mysym == nullptr || *mysym != sym) return; Symbols.Remove(sym->SymbolName); + delete sym; } //========================================================================== @@ -265,20 +252,20 @@ void PSymbolTable::RemoveSymbol(PSymbol *sym) // //========================================================================== -PSymbol *PSymbolTable::ReplaceSymbol(PSymbol *newsym) +void PSymbolTable::ReplaceSymbol(PSymbol *newsym) { // If a symbol with a matching name exists, take its place and return it. PSymbol **symslot = Symbols.CheckKey(newsym->SymbolName); if (symslot != nullptr) { PSymbol *oldsym = *symslot; + delete oldsym; *symslot = newsym; - return oldsym; } // Else, just insert normally and return nullptr since there was no // symbol to replace. + newsym->Release(); // no more GC, please! Symbols.Insert(newsym->SymbolName, newsym); - return nullptr; } //========================================================================== @@ -306,18 +293,6 @@ PNamespace::PNamespace(int filenum, PNamespace *parent) // //========================================================================== -size_t PNamespace::PropagateMark() -{ - GC::Mark(Parent); - return Symbols.MarkSymbols() + 1; -} - -//========================================================================== -// -// -// -//========================================================================== - FNamespaceManager::FNamespaceManager() { GlobalNamespace = nullptr; @@ -370,6 +345,7 @@ size_t FNamespaceManager::MarkSymbols() void FNamespaceManager::ReleaseSymbols() { + RemoveSymbols(); GlobalNamespace = nullptr; AllNamespaces.Clear(); } diff --git a/src/scripting/symbols.h b/src/scripting/symbols.h index af5bfb80e..3c9d1f8f3 100644 --- a/src/scripting/symbols.h +++ b/src/scripting/symbols.h @@ -192,8 +192,6 @@ struct PSymbolTable PSymbolTable(PSymbolTable *parent); ~PSymbolTable(); - size_t MarkSymbols(); - // Sets the table to use for searches if this one doesn't contain the // requested symbol. void SetParentTable (PSymbolTable *parent); @@ -218,7 +216,7 @@ struct PSymbolTable // Similar to AddSymbol but always succeeds. Returns the symbol that used // to be in the table with this name, if any. - PSymbol *ReplaceSymbol(PSymbol *sym); + void ReplaceSymbol(PSymbol *sym); void RemoveSymbol(PSymbol *sym); @@ -255,7 +253,6 @@ public: PNamespace() {} PNamespace(int filenum, PNamespace *parent); - size_t PropagateMark(); }; struct FNamespaceManager