Skip to content

Commit d73fecd

Browse files
authored
Merge pull request #180 from hamishknight/take-a-hint
Reduce memory consumption for OutOfDateTriggerHints
2 parents 00b81b1 + 9afa14e commit d73fecd

File tree

4 files changed

+88
-114
lines changed

4 files changed

+88
-114
lines changed

include/IndexStoreDB/Index/IndexSystemDelegate.h

Lines changed: 21 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -20,49 +20,36 @@
2020

2121
namespace IndexStoreDB {
2222
namespace index {
23-
struct StoreUnitInfo;
23+
struct StoreUnitInfo;
24+
class OutOfDateFileTrigger;
2425

25-
/// Used to document why a unit was considered out-of-date.
26-
/// Primarily used for logging/debugging purposes.
27-
class OutOfDateTriggerHint {
28-
public:
29-
virtual ~OutOfDateTriggerHint() {}
30-
virtual std::string originalFileTrigger() = 0;
31-
virtual std::string description() = 0;
26+
typedef std::shared_ptr<OutOfDateFileTrigger> OutOfDateFileTriggerRef;
3227

33-
private:
34-
virtual void _anchor();
35-
};
36-
typedef std::shared_ptr<OutOfDateTriggerHint> OutOfDateTriggerHintRef;
37-
38-
class DependentFileOutOfDateTriggerHint : public OutOfDateTriggerHint {
28+
/// Records a known out-of-date file path for a unit, along with its
29+
/// modification time. This is used to provide IndexDelegate with information
30+
/// about the file that triggered the unit to become out-of-date.
31+
class OutOfDateFileTrigger final {
3932
std::string FilePath;
33+
llvm::sys::TimePoint<> ModTime;
4034

4135
public:
42-
explicit DependentFileOutOfDateTriggerHint(StringRef filePath) : FilePath(filePath) {}
36+
explicit OutOfDateFileTrigger(StringRef filePath,
37+
llvm::sys::TimePoint<> modTime)
38+
: FilePath(filePath), ModTime(modTime) {}
4339

44-
static OutOfDateTriggerHintRef create(StringRef filePath) {
45-
return std::make_shared<DependentFileOutOfDateTriggerHint>(filePath);
40+
static OutOfDateFileTriggerRef create(StringRef filePath,
41+
llvm::sys::TimePoint<> modTime) {
42+
return std::make_shared<OutOfDateFileTrigger>(filePath, modTime);
4643
}
4744

48-
virtual std::string originalFileTrigger() override;
49-
virtual std::string description() override;
50-
};
45+
llvm::sys::TimePoint<> getModTime() const { return ModTime; }
5146

52-
class DependentUnitOutOfDateTriggerHint : public OutOfDateTriggerHint {
53-
std::string UnitName;
54-
OutOfDateTriggerHintRef DepHint;
55-
56-
public:
57-
DependentUnitOutOfDateTriggerHint(StringRef unitName, OutOfDateTriggerHintRef depHint)
58-
: UnitName(unitName), DepHint(std::move(depHint)) {}
59-
60-
static OutOfDateTriggerHintRef create(StringRef unitName, OutOfDateTriggerHintRef depHint) {
61-
return std::make_shared<DependentUnitOutOfDateTriggerHint>(unitName, std::move(depHint));
62-
}
47+
/// Returns a reference to the stored file path. Note this has the same
48+
/// lifetime as the trigger.
49+
StringRef getPathRef() const { return FilePath; }
6350

64-
virtual std::string originalFileTrigger() override;
65-
virtual std::string description() override;
51+
std::string getPath() const { return FilePath; }
52+
std::string description() { return FilePath; }
6653
};
6754

6855
class INDEXSTOREDB_EXPORT IndexSystemDelegate {
@@ -78,8 +65,7 @@ class INDEXSTOREDB_EXPORT IndexSystemDelegate {
7865
virtual void processedStoreUnit(StoreUnitInfo unitInfo) {}
7966

8067
virtual void unitIsOutOfDate(StoreUnitInfo unitInfo,
81-
llvm::sys::TimePoint<> outOfDateModTime,
82-
OutOfDateTriggerHintRef hint,
68+
OutOfDateFileTriggerRef trigger,
8369
bool synchronous = false) {}
8470

8571
private:

lib/CIndexStoreDB/CIndexStoreDB.cpp

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,17 +80,18 @@ class BlockIndexSystemDelegate: public IndexSystemDelegate {
8080
callback(&event);
8181
}
8282

83-
void unitIsOutOfDate(StoreUnitInfo unitInfo,
84-
llvm::sys::TimePoint<> outOfDateModTime,
85-
OutOfDateTriggerHintRef hint,
83+
void unitIsOutOfDate(StoreUnitInfo unitInfo, OutOfDateFileTriggerRef trigger,
8684
bool synchronous) override {
87-
DelegateEvent event{INDEXSTOREDB_EVENT_UNIT_OUT_OF_DATE, 0,
88-
&unitInfo,
89-
(uint64_t)std::chrono::duration_cast<std::chrono::nanoseconds>(outOfDateModTime.time_since_epoch()).count(),
90-
hint->originalFileTrigger(),
91-
hint->description(),
92-
synchronous
93-
};
85+
DelegateEvent event{
86+
INDEXSTOREDB_EVENT_UNIT_OUT_OF_DATE,
87+
0,
88+
&unitInfo,
89+
(uint64_t)std::chrono::duration_cast<std::chrono::nanoseconds>(
90+
trigger->getModTime().time_since_epoch())
91+
.count(),
92+
trigger->getPath(),
93+
trigger->description(),
94+
synchronous};
9495
callback(&event);
9596
}
9697
};

lib/Index/IndexDatastore.cpp

Lines changed: 53 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "IndexStoreDB/Support/Logging.h"
2828
#include "indexstore/IndexStoreCXX.h"
2929
#include "llvm/ADT/ArrayRef.h"
30+
#include "llvm/ADT/DenseMap.h"
3031
#include "llvm/ADT/STLExtras.h"
3132
#include "llvm/ADT/StringMap.h"
3233
#include "llvm/ADT/StringRef.h"
@@ -147,8 +148,7 @@ class StoreUnitRepo : public std::enable_shared_from_this<StoreUnitRepo> {
147148
void removeUnitMonitor(IDCode unitCode);
148149

149150
void onUnitOutOfDate(IDCode unitCode, StringRef unitName,
150-
sys::TimePoint<> outOfDateModTime,
151-
OutOfDateTriggerHintRef hint,
151+
OutOfDateFileTriggerRef trigger,
152152
bool synchronous = false);
153153
void onFSEvent(std::vector<std::string> parentPaths);
154154
void checkUnitContainingFileIsOutOfDate(StringRef file);
@@ -184,23 +184,20 @@ class IndexDatastoreImpl {
184184
};
185185

186186
class UnitMonitor {
187-
struct OutOfDateTrigger {
188-
OutOfDateTriggerHintRef hint;
189-
sys::TimePoint<> outOfDateModTime;
190-
191-
std::string getTriggerFilePath() const {
192-
return hint->originalFileTrigger();
193-
}
194-
};
195-
196187
std::weak_ptr<StoreUnitRepo> UnitRepo;
197188
IDCode UnitCode;
198189
std::string UnitName;
199190
sys::TimePoint<> ModTime;
200191

201192
mutable llvm::sys::Mutex StateMtx;
202-
/// Map of out-of-date file path to its associated info.
203-
StringMap<OutOfDateTrigger> OutOfDateTriggers;
193+
194+
/// Map of out-of-date file paths to their associated info. Note that the
195+
/// StringRef key references the string in the trigger, so must not outlive
196+
/// it. Access to this map should be guarded by \c StateMtx.
197+
llvm::DenseMap<StringRef, OutOfDateFileTriggerRef> OutOfDateTriggers;
198+
199+
/// Retrieves an unordered list of out-of-date trigger files.
200+
std::vector<OutOfDateFileTriggerRef> getUnorderedOutOfDateTriggers() const;
204201

205202
public:
206203
UnitMonitor(std::shared_ptr<StoreUnitRepo> unitRepo);
@@ -217,10 +214,8 @@ class UnitMonitor {
217214
StringRef getUnitName() const { return UnitName; }
218215
sys::TimePoint<> getModTime() const { return ModTime; }
219216

220-
std::vector<OutOfDateTrigger> getOutOfDateTriggers() const;
221-
222217
void checkForOutOfDate(sys::TimePoint<> outOfDateModTime, StringRef filePath, bool synchronous=false);
223-
void markOutOfDate(sys::TimePoint<> outOfDateModTime, OutOfDateTriggerHintRef hint, bool synchronous=false);
218+
void markOutOfDate(OutOfDateFileTriggerRef trigger, bool synchronous = false);
224219

225220
static std::pair<StringRef, sys::TimePoint<>> getMostRecentModTime(ArrayRef<StringRef> filePaths);
226221
static sys::TimePoint<> getModTimeForOutOfDateCheck(StringRef filePath);
@@ -826,8 +821,7 @@ void StoreUnitRepo::removeUnitMonitor(IDCode unitCode) {
826821
}
827822

828823
void StoreUnitRepo::onUnitOutOfDate(IDCode unitCode, StringRef unitName,
829-
sys::TimePoint<> outOfDateModTime,
830-
OutOfDateTriggerHintRef hint,
824+
OutOfDateFileTriggerRef trigger,
831825
bool synchronous) {
832826
CanonicalFilePath MainFilePath;
833827
std::string OutFileIdentifier;
@@ -859,15 +853,13 @@ void StoreUnitRepo::onUnitOutOfDate(IDCode unitCode, StringRef unitName,
859853
CurrModTime,
860854
SymProviderKind
861855
};
862-
Delegate->unitIsOutOfDate(unitInfo, outOfDateModTime, hint, synchronous);
856+
Delegate->unitIsOutOfDate(unitInfo, trigger, synchronous);
863857
}
864858

865859
for (IDCode depUnit : dependentUnits) {
866860
if (auto monitor = getUnitMonitor(depUnit)) {
867-
if (monitor->getModTime() < outOfDateModTime)
868-
monitor->markOutOfDate(outOfDateModTime,
869-
DependentUnitOutOfDateTriggerHint::create(unitName, hint),
870-
synchronous);
861+
if (monitor->getModTime() < trigger->getModTime())
862+
monitor->markOutOfDate(trigger, synchronous);
871863
}
872864
}
873865
}
@@ -957,12 +949,9 @@ void UnitMonitor::initialize(IDCode unitCode,
957949
this->ModTime = modTime;
958950
for (IDCode unitDepCode : userUnitDepends) {
959951
if (auto depMonitor = unitRepo->getUnitMonitor(unitDepCode)) {
960-
for (const auto &trigger : depMonitor->getOutOfDateTriggers()) {
961-
if (trigger.outOfDateModTime > modTime) {
962-
markOutOfDate(trigger.outOfDateModTime,
963-
DependentUnitOutOfDateTriggerHint::create(depMonitor->getUnitName(),
964-
trigger.hint));
965-
}
952+
for (const auto &trigger : depMonitor->getUnorderedOutOfDateTriggers()) {
953+
if (trigger->getModTime() > modTime)
954+
markOutOfDate(trigger);
966955
}
967956
}
968957
}
@@ -975,43 +964,63 @@ void UnitMonitor::initialize(IDCode unitCode,
975964
}
976965
auto mostRecentFileAndTime = getMostRecentModTime(filePaths);
977966
if (mostRecentFileAndTime.second > modTime) {
978-
markOutOfDate(mostRecentFileAndTime.second, DependentFileOutOfDateTriggerHint::create(mostRecentFileAndTime.first));
967+
auto trigger = OutOfDateFileTrigger::create(mostRecentFileAndTime.first,
968+
mostRecentFileAndTime.second);
969+
markOutOfDate(trigger);
979970
}
980971
}
981972
}
982973

983974
UnitMonitor::~UnitMonitor() {}
984975

985-
std::vector<UnitMonitor::OutOfDateTrigger> UnitMonitor::getOutOfDateTriggers() const {
976+
std::vector<OutOfDateFileTriggerRef>
977+
UnitMonitor::getUnorderedOutOfDateTriggers() const {
986978
sys::ScopedLock L(StateMtx);
987-
std::vector<OutOfDateTrigger> triggers;
979+
std::vector<OutOfDateFileTriggerRef> triggers;
988980
for (const auto &entry : OutOfDateTriggers) {
989-
triggers.push_back(entry.getValue());
981+
triggers.push_back(entry.second);
990982
}
991983
return triggers;
992984
}
993985

994-
void UnitMonitor::checkForOutOfDate(sys::TimePoint<> outOfDateModTime, StringRef filePath, bool synchronous) {
986+
void UnitMonitor::checkForOutOfDate(sys::TimePoint<> outOfDateModTime,
987+
StringRef filePath, bool synchronous) {
995988
sys::ScopedLock L(StateMtx);
996989
auto findIt = OutOfDateTriggers.find(filePath);
997-
if (findIt != OutOfDateTriggers.end() && findIt->getValue().outOfDateModTime >= outOfDateModTime) {
990+
if (findIt != OutOfDateTriggers.end() &&
991+
findIt->second->getModTime() >= outOfDateModTime) {
998992
return; // already marked as out-of-date related to this trigger file.
999993
}
1000-
if (ModTime < outOfDateModTime)
1001-
markOutOfDate(outOfDateModTime, DependentFileOutOfDateTriggerHint::create(filePath), synchronous);
994+
if (ModTime < outOfDateModTime) {
995+
markOutOfDate(OutOfDateFileTrigger::create(filePath, outOfDateModTime),
996+
synchronous);
997+
}
1002998
}
1003999

1004-
void UnitMonitor::markOutOfDate(sys::TimePoint<> outOfDateModTime, OutOfDateTriggerHintRef hint, bool synchronous) {
1000+
void UnitMonitor::markOutOfDate(OutOfDateFileTriggerRef trigger,
1001+
bool synchronous) {
10051002
{
1003+
// Note we have to be careful with the memory management here since the
1004+
// key for OutOfDateTriggers is a reference into the stored trigger value.
10061005
sys::ScopedLock L(StateMtx);
1007-
OutOfDateTrigger trigger{ hint, outOfDateModTime};
1008-
auto &entry = OutOfDateTriggers[trigger.getTriggerFilePath()];
1009-
if (entry.outOfDateModTime >= outOfDateModTime)
1010-
return; // already marked as out-of-date related to this trigger file.
1011-
entry = trigger;
1006+
auto iterAndInserted =
1007+
OutOfDateTriggers.try_emplace(trigger->getPathRef(), trigger);
1008+
if (!iterAndInserted.second) {
1009+
// If we have the same or newer mod time for this trigger already stored,
1010+
// we've seen it before, and have already informed the delegate that the
1011+
// unit is out of date.
1012+
auto iter = iterAndInserted.first;
1013+
if (iter->second->getModTime() >= trigger->getModTime())
1014+
return;
1015+
1016+
// We have a newer mod time for the file, update our trigger, and inform
1017+
// the delegate that the unit is out of date. Note we need to overwrite
1018+
// the key as well since it references the path stored in the trigger.
1019+
*iter = {trigger->getPathRef(), trigger};
1020+
}
10121021
}
10131022
if (auto localUnitRepo = UnitRepo.lock())
1014-
localUnitRepo->onUnitOutOfDate(UnitCode, UnitName, outOfDateModTime, hint, synchronous);
1023+
localUnitRepo->onUnitOutOfDate(UnitCode, UnitName, trigger, synchronous);
10151024
}
10161025

10171026
std::pair<StringRef, sys::TimePoint<>> UnitMonitor::getMostRecentModTime(ArrayRef<StringRef> filePaths) {

lib/Index/IndexSystem.cpp

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -95,20 +95,19 @@ class AsyncIndexDelegate : public IndexSystemDelegate {
9595
}
9696

9797
virtual void unitIsOutOfDate(StoreUnitInfo unitInfo,
98-
llvm::sys::TimePoint<> outOfDateModTime,
99-
OutOfDateTriggerHintRef hint,
98+
OutOfDateFileTriggerRef trigger,
10099
bool synchronous) override {
101100
if (synchronous) {
102101
Queue.dispatchSync([&]{
103102
for (auto &other : Others)
104-
other->unitIsOutOfDate(std::move(unitInfo), outOfDateModTime, hint, true);
103+
other->unitIsOutOfDate(std::move(unitInfo), trigger, true);
105104
});
106105
return;
107106
}
108107

109108
Queue.dispatch([=]{
110109
for (auto &other : Others)
111-
other->unitIsOutOfDate(std::move(unitInfo), outOfDateModTime, hint, false);
110+
other->unitIsOutOfDate(std::move(unitInfo), trigger, false);
112111
});
113112
}
114113

@@ -631,27 +630,6 @@ bool IndexSystemImpl::foreachUnitTestSymbol(function_ref<bool(SymbolOccurrenceRe
631630
// IndexSystem
632631
//===----------------------------------------------------------------------===//
633632

634-
void OutOfDateTriggerHint::_anchor() {}
635-
636-
std::string DependentFileOutOfDateTriggerHint::originalFileTrigger() {
637-
return FilePath;
638-
}
639-
640-
std::string DependentFileOutOfDateTriggerHint::description() {
641-
return FilePath;
642-
}
643-
644-
std::string DependentUnitOutOfDateTriggerHint::originalFileTrigger() {
645-
return DepHint->originalFileTrigger();
646-
}
647-
648-
std::string DependentUnitOutOfDateTriggerHint::description() {
649-
std::string desc;
650-
llvm::raw_string_ostream OS(desc);
651-
OS << "unit(" << UnitName << ") -> " << DepHint->description();
652-
return desc;
653-
}
654-
655633
void IndexSystemDelegate::anchor() {}
656634

657635
std::shared_ptr<IndexSystem>

0 commit comments

Comments
 (0)