Skip to content

Commit e1be0dd

Browse files
committed
[Support][JSON] Use std::map for object storage
`llvm::DenseMap` is not suitable for the key-value `Storage` inside `llvm::json::Object`. Use `std::map` instead to optimize memory usage. `llvm::DenseMap` is optimized for mapping small keys and values (pointers), and it pre-allocates 64 buckets by default. `llvm::json::ObjectKey` is 24 bytes in size, and `llvm::json::Value` is 40 bytes. Currently, the JSON parser allocates 4KB of memory for each JSON object. In practice, most JSON objects contain only a handful of fields, and likely to have lists of many small objects. This is a significant waste of memory.
1 parent fae64ad commit e1be0dd

File tree

5 files changed

+15
-15
lines changed

5 files changed

+15
-15
lines changed

clang/lib/Basic/DarwinSDKInfo.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ std::optional<VersionTuple> DarwinSDKInfo::RelatedTargetVersionMapping::map(
2525
return MaximumValue;
2626
auto KV = Mapping.find(Key.normalize());
2727
if (KV != Mapping.end())
28-
return KV->getSecond();
28+
return KV->second;
2929
// If no exact entry found, try just the major key version. Only do so when
3030
// a minor version number is present, to avoid recursing indefinitely into
3131
// the major-only check.
@@ -43,10 +43,10 @@ DarwinSDKInfo::RelatedTargetVersionMapping::parseJSON(
4343
VersionTuple MinValue = Min;
4444
llvm::DenseMap<VersionTuple, VersionTuple> Mapping;
4545
for (const auto &KV : Obj) {
46-
if (auto Val = KV.getSecond().getAsString()) {
46+
if (auto Val = KV.second.getAsString()) {
4747
llvm::VersionTuple KeyVersion;
4848
llvm::VersionTuple ValueVersion;
49-
if (KeyVersion.tryParse(KV.getFirst()) || ValueVersion.tryParse(*Val))
49+
if (KeyVersion.tryParse(KV.first) || ValueVersion.tryParse(*Val))
5050
return std::nullopt;
5151
Mapping[KeyVersion.normalize()] = ValueVersion;
5252
if (KeyVersion < Min)
@@ -113,12 +113,12 @@ DarwinSDKInfo::parseDarwinSDKSettingsJSON(const llvm::json::Object *Obj) {
113113
// FIXME: Generalize this out beyond iOS-deriving targets.
114114
// Look for ios_<targetos> version mapping for targets that derive from ios.
115115
for (const auto &KV : *VM) {
116-
auto Pair = StringRef(KV.getFirst()).split("_");
116+
auto Pair = StringRef(KV.first).split("_");
117117
if (Pair.first.compare_insensitive("ios") == 0) {
118118
llvm::Triple TT(llvm::Twine("--") + Pair.second.lower());
119119
if (TT.getOS() != llvm::Triple::UnknownOS) {
120120
auto Mapping = RelatedTargetVersionMapping::parseJSON(
121-
*KV.getSecond().getAsObject(), *MaximumDeploymentVersion);
121+
*KV.second.getAsObject(), *MaximumDeploymentVersion);
122122
if (Mapping)
123123
VersionMappings[OSEnvPair(llvm::Triple::IOS,
124124
llvm::Triple::UnknownEnvironment,

clang/tools/clang-installapi/Options.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,8 @@ getArgListFromJSON(const StringRef Input, llvm::opt::OptTable *Table,
8484
return llvm::opt::InputArgList();
8585

8686
for (const auto &KV : *Root) {
87-
const Array *ArgList = KV.getSecond().getAsArray();
88-
std::string Label = "-X" + KV.getFirst().str();
87+
const Array *ArgList = KV.second.getAsArray();
88+
std::string Label = "-X" + KV.first.str();
8989
if (!ArgList)
9090
return make_error<TextAPIError>(TextAPIErrorCode::InvalidInputFormat);
9191
for (auto Arg : *ArgList) {

clang/unittests/Basic/SarifTest.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ TEST_F(SarifDocumentWriterTest, canCreateEmptyDocument) {
8383
const llvm::json::Object &EmptyDoc = Writer.createDocument();
8484
std::vector<StringRef> Keys(EmptyDoc.size());
8585
std::transform(EmptyDoc.begin(), EmptyDoc.end(), Keys.begin(),
86-
[](auto Item) { return Item.getFirst(); });
86+
[](auto Item) { return Item.first; });
8787

8888
// THEN:
8989
ASSERT_THAT(Keys, testing::UnorderedElementsAre("$schema", "version"));

llvm/include/llvm/Support/JSON.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,9 @@ class Value;
9494
template <typename T> Value toJSON(const std::optional<T> &Opt);
9595

9696
/// An Object is a JSON object, which maps strings to heterogenous JSON values.
97-
/// It simulates DenseMap<ObjectKey, Value>. ObjectKey is a maybe-owned string.
97+
/// ObjectKey is a maybe-owned string.
9898
class Object {
99-
using Storage = DenseMap<ObjectKey, Value, llvm::DenseMapInfo<StringRef>>;
99+
using Storage = std::map<ObjectKey, Value>;
100100
Storage M;
101101

102102
public:
@@ -133,8 +133,8 @@ class Object {
133133
bool erase(StringRef K);
134134
void erase(iterator I) { M.erase(I); }
135135

136-
iterator find(StringRef K) { return M.find_as(K); }
137-
const_iterator find(StringRef K) const { return M.find_as(K); }
136+
iterator find(const ObjectKey &K) { return M.find(K); }
137+
const_iterator find(const ObjectKey &K) const { return M.find(K); }
138138
// operator[] acts as if Value was default-constructible as null.
139139
LLVM_ABI Value &operator[](const ObjectKey &K);
140140
LLVM_ABI Value &operator[](ObjectKey &&K);
@@ -649,7 +649,7 @@ inline Object::Object(std::initializer_list<KV> Properties) {
649649
for (const auto &P : Properties) {
650650
auto R = try_emplace(P.K, nullptr);
651651
if (R.second)
652-
R.first->getSecond().moveFrom(std::move(P.V));
652+
R.first->second.moveFrom(std::move(P.V));
653653
}
654654
}
655655
inline std::pair<Object::iterator, bool> Object::insert(KV E) {

llvm/lib/Support/JSON.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ namespace llvm {
2222
namespace json {
2323

2424
Value &Object::operator[](const ObjectKey &K) {
25-
return try_emplace(K, nullptr).first->getSecond();
25+
return try_emplace(K, nullptr).first->second;
2626
}
2727
Value &Object::operator[](ObjectKey &&K) {
28-
return try_emplace(std::move(K), nullptr).first->getSecond();
28+
return try_emplace(std::move(K), nullptr).first->second;
2929
}
3030
Value *Object::get(StringRef K) {
3131
auto I = find(K);

0 commit comments

Comments
 (0)