Skip to content

Commit 10f43a8

Browse files
bnbarhambnbarham
committed
[Index] Convert to hashing to HashBuilder with BLAKE3
`Hashing.h` is non-deterministic between runs. Update the index hashing to use BLAKE3 for the record hash. xxhash is faster in benchmarks that I've found, but there's no easy `HashBuilder` option for it today.
1 parent 008e6ee commit 10f43a8

File tree

6 files changed

+107
-65
lines changed

6 files changed

+107
-65
lines changed

‎lib/Index/IndexRecord.cpp‎

Lines changed: 54 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@
3535
#include "clang/Index/IndexingAction.h"
3636
#include "clang/Lex/Preprocessor.h"
3737
#include "clang/Serialization/ASTReader.h"
38+
#include "llvm/Support/BLAKE3.h"
3839
#include "llvm/Support/Error.h"
40+
#include "llvm/Support/HashBuilder.h"
3941
#include "llvm/Support/Path.h"
4042

4143
using namespace swift;
@@ -50,29 +52,48 @@ using clang::index::SymbolRoleSet;
5052
//===----------------------------------------------------------------------===//
5153

5254
namespace {
55+
56+
using HashBuilderTy =
57+
llvm::HashBuilder<llvm::TruncatedBLAKE3<8>, llvm::endianness::little>;
58+
5359
class SymbolTracker {
5460
public:
5561
struct SymbolRelation {
5662
size_t symbolIndex;
5763
SymbolRoleSet roles;
5864

59-
llvm::hash_code hash() const { return llvm::hash_combine(symbolIndex, roles); }
65+
SymbolRelation(size_t symbolIndex, SymbolRoleSet roles)
66+
: symbolIndex(symbolIndex), roles(roles) {}
67+
68+
void hash(HashBuilderTy &HashBuilder) {
69+
HashBuilder.add(symbolIndex);
70+
HashBuilder.add(roles);
71+
}
6072
};
73+
6174
struct SymbolOccurrence {
6275
size_t symbolIndex;
6376
SymbolRoleSet roles;
6477
unsigned line;
6578
unsigned column;
6679
SmallVector<SymbolRelation, 3> related;
6780

68-
llvm::hash_code hash() const {
69-
auto hash = llvm::hash_combine(symbolIndex, roles, line, column);
81+
SymbolOccurrence(size_t symbolIndex, SymbolRoleSet roles, unsigned line,
82+
unsigned column, SmallVector<SymbolRelation, 3> related)
83+
: symbolIndex(symbolIndex), roles(roles), line(line), column(column),
84+
related(std::move(related)) {}
85+
86+
void hash(HashBuilderTy &HashBuilder) {
87+
HashBuilder.add(symbolIndex);
88+
HashBuilder.add(roles);
89+
HashBuilder.add(line);
90+
HashBuilder.add(column);
7091
for (auto &relation : related) {
71-
hash = llvm::hash_combine(hash, relation.hash());
92+
relation.hash(HashBuilder);
7293
}
73-
return hash;
7494
}
7595
};
96+
7697
struct Symbol {
7798
StringRef name;
7899
StringRef USR;
@@ -81,12 +102,19 @@ class SymbolTracker {
81102
SymbolInfo symInfo;
82103
unsigned isTestCandidate : 1;
83104

84-
llvm::hash_code hash() const {
85-
return llvm::hash_combine(
86-
name, USR, group,
87-
static_cast<unsigned>(symInfo.Kind),
88-
static_cast<unsigned>(symInfo.SubKind),
89-
symInfo.Properties, isTestCandidate);
105+
Symbol(StringRef name, StringRef usr, StringRef group, SymbolInfo symInfo,
106+
bool isTestCandidate)
107+
: name(name), USR(usr), group(group), symInfo(std::move(symInfo)),
108+
isTestCandidate(isTestCandidate) {}
109+
110+
void hash(HashBuilderTy &HashBuilder) {
111+
HashBuilder.add(name);
112+
HashBuilder.add(USR);
113+
HashBuilder.add(group);
114+
HashBuilder.add(symInfo.Kind);
115+
HashBuilder.add(symInfo.SubKind);
116+
HashBuilder.add(symInfo.Properties);
117+
HashBuilder.add(isTestCandidate);
90118
}
91119
};
92120

@@ -114,13 +142,9 @@ class SymbolTracker {
114142
auto pair = USRToSymbol.insert(std::make_pair(indexSym.USR.data(),
115143
symbols.size()));
116144
if (pair.second) {
117-
Symbol symbol{indexSym.name,
118-
indexSym.USR,
119-
indexSym.group,
120-
indexSym.symInfo,
121-
0};
122-
recordHash = llvm::hash_combine(recordHash, symbol.hash());
123-
symbols.push_back(std::move(symbol));
145+
symbols.emplace_back(indexSym.name, indexSym.USR, indexSym.group,
146+
indexSym.symInfo, 0);
147+
symbols.back().hash(HashBuilder);
124148
}
125149

126150
return pair.first->second;
@@ -131,26 +155,28 @@ class SymbolTracker {
131155

132156
SmallVector<SymbolRelation, 3> relations;
133157
for(IndexRelation indexRel: indexOccur.Relations) {
134-
relations.push_back({addSymbol(indexRel), indexRel.roles});
158+
relations.emplace_back(addSymbol(indexRel), indexRel.roles);
135159
}
136160

137-
occurrences.push_back({/*symbolIndex=*/addSymbol(indexOccur),
138-
indexOccur.roles,
139-
indexOccur.line,
140-
indexOccur.column,
141-
std::move(relations)});
142-
143-
recordHash = llvm::hash_combine(recordHash, occurrences.back().hash());
161+
occurrences.emplace_back(addSymbol(indexOccur), indexOccur.roles,
162+
indexOccur.line, indexOccur.column,
163+
std::move(relations));
164+
occurrences.back().hash(HashBuilder);
144165
}
145166

146-
llvm::hash_code hashRecord() const { return recordHash; }
167+
uint64_t hashRecord() {
168+
std::array<uint8_t, 8> recordHashArr = HashBuilder.final();
169+
uint64_t recordHash = 0;
170+
std::memcpy(&recordHash, recordHashArr.data(), recordHashArr.size());
171+
return recordHash;
172+
}
147173

148174
private:
149175
llvm::DenseMap<const char *, size_t> USRToSymbol;
150176
std::vector<Symbol> symbols;
151177
std::vector<SymbolOccurrence> occurrences;
152178
bool sorted = false;
153-
llvm::hash_code recordHash = 0;
179+
HashBuilderTy HashBuilder;
154180
};
155181

156182
class IndexRecordingConsumer : public IndexDataConsumer {

‎test/Index/Store/cross-import-overlay.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ from__ABAdditionsCAdditions()
2525
//
2626
// RUN: %FileCheck %s --input-file %t/units --check-prefix=MAIN
2727
// MAIN: module-name: cross_import_overlay
28-
// MAIN: out-file: {{.*}}/file1.o
28+
// MAIN: out-file: {{.*}}{{/|\\}}file1.o
2929
// MAIN-NEXT: target: {{.*}}
3030
// MAIN-NEXT: is-debug: 1
3131
// MAIN-NEXT: DEPEND START

‎test/Index/Store/unit-multiple-sourcefiles-remapped.swift‎

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,39 +2,50 @@
22

33
// RUN: %empty-directory(%t)
44
// RUN: touch %t/s1.swift %t/s2.swift
5-
// RUN: %target-swift-frontend -index-store-path %t/idx -file-prefix-map %t=REMAPPED_OUT_DIR -primary-file %t/s1.swift %t/s2.swift -o %t/s1.o -c -module-name main -emit-module -emit-module-path %t/s1.swiftmodule
6-
// RUN: %target-swift-frontend -index-store-path %t/idx -file-prefix-map %t=REMAPPED_OUT_DIR %t/s1.swift -primary-file %t/s2.swift -o %t/s2.o -c -module-name main -emit-module -emit-module-path %t/s2.swiftmodule
7-
// RUN: %target-swift-frontend -index-store-path %t/idx -file-prefix-map %t=REMAPPED_OUT_DIR %t/s1.swiftmodule %t/s2.swiftmodule -emit-module -o %t/main.swiftmodule -module-name main
8-
// RUN: c-index-test core -print-unit %t/idx | %FileCheck %s
5+
6+
// RUN: echo "-----separate-----" > %t/units.out
7+
// RUN: %target-swift-frontend -index-store-path %t/idx1 -file-prefix-map %t=REMAPPED_OUT_DIR -primary-file %t/s1.swift %t/s2.swift -o %t/s1.o -c -module-name main -emit-module -emit-module-path %t/s1.swiftmodule
8+
// RUN: %target-swift-frontend -index-store-path %t/idx1 -file-prefix-map %t=REMAPPED_OUT_DIR %t/s1.swift -primary-file %t/s2.swift -o %t/s2.o -c -module-name main -emit-module -emit-module-path %t/s2.swiftmodule
9+
// RUN: %target-swift-frontend -index-store-path %t/idx1 -file-prefix-map %t=REMAPPED_OUT_DIR %t/s1.swiftmodule %t/s2.swiftmodule -emit-module -o %t/main.swiftmodule -module-name main
10+
// RUN: c-index-test core -print-unit %t/idx1 >> %t/units.out
911

1012
//===--- Building source files together (e.g. WMO)
1113

12-
// RUN: %empty-directory(%t)
13-
// RUN: touch %t/s1.swift %t/s2.swift
14-
// RUN: %target-swift-frontend -index-store-path %t/idx -file-prefix-map %t=REMAPPED_OUT_DIR %t/s1.swift %t/s2.swift -o %t/s1.o -o %t/s2.o -c -module-name main -emit-module -emit-module-path %t/main.swiftmodule
15-
// RUN: c-index-test core -print-unit %t/idx | %FileCheck %s
14+
// RUN: echo "-----together-----" >> %t/units.out
15+
// RUN: %target-swift-frontend -index-store-path %t/idx2 -file-prefix-map %t=REMAPPED_OUT_DIR %t/s1.swift %t/s2.swift -o %t/s1.o -o %t/s2.o -c -module-name main -emit-module -emit-module-path %t/main.swiftmodule
16+
// RUN: c-index-test core -print-unit %t/idx2 >> %t/units.out
1617

1718
//===--- Building separately but with relative paths for the source file inputs
1819

19-
// RUN: %empty-directory(%t)
2020
// RUN: cd %t
21-
// RUN: touch %t/s1.swift %t/s2.swift
22-
// RUN: %target-swift-frontend -index-store-path idx -file-prefix-map %t=REMAPPED_OUT_DIR -primary-file s1.swift s2.swift -o s1.o -c -module-name main -emit-module -emit-module-path s1.swiftmodule
23-
// RUN: %target-swift-frontend -index-store-path idx -file-prefix-map %t=REMAPPED_OUT_DIR s1.swift -primary-file s2.swift -o s2.o -c -module-name main -emit-module -emit-module-path s2.swiftmodule
24-
// RUN: %target-swift-frontend -index-store-path idx -file-prefix-map %t=REMAPPED_OUT_DIR s1.swiftmodule s2.swiftmodule -emit-module -o main.swiftmodule -module-name main
25-
// RUN: c-index-test core -print-unit idx | %FileCheck %s
26-
// CHECK-NOT: main.swiftmodule-{{[A-Z0-9]*}}
21+
// RUN: echo "-----together-relative-----" >> %t/units.out
22+
// RUN: %target-swift-frontend -index-store-path idx3 -file-prefix-map %t=REMAPPED_OUT_DIR -primary-file s1.swift s2.swift -o s1.o -c -module-name main -emit-module -emit-module-path s1.swiftmodule
23+
// RUN: %target-swift-frontend -index-store-path idx3 -file-prefix-map %t=REMAPPED_OUT_DIR s1.swift -primary-file s2.swift -o s2.o -c -module-name main -emit-module -emit-module-path s2.swiftmodule
24+
// RUN: %target-swift-frontend -index-store-path idx3 -file-prefix-map %t=REMAPPED_OUT_DIR s1.swiftmodule s2.swiftmodule -emit-module -o main.swiftmodule -module-name main
25+
// RUN: c-index-test core -print-unit idx3 >> %t/units.out
26+
27+
// RUN: %FileCheck %s --dump-input-filter all < %t/units.out
2728

28-
// CHECK: s1.o-{{2LQAU7D8TZHZ8|2RHC8ZJFDYDW4}}
29+
// CHECK: -----separate-----
30+
// CHECK: s1.o-[[S1_HASH:.*$]]
2931
// CHECK: --------
3032
// CHECK: out-file: REMAPPED_OUT_DIR{{/|\\}}s1.o
31-
// CHECK: DEPEND START
32-
// CHECK: Unit | system | {{.*}}Swift.swiftmodule
33-
// CHECK: DEPEND END
33+
// CHECK: s2.o-[[S2_HASH:.*$]]
34+
// CHECK: --------
35+
// CHECK: out-file: REMAPPED_OUT_DIR{{/|\\}}s2.o
3436

35-
// CHECK: s2.o-{{2OIL2LG8UULK6|15MCL6ZLKZKNL}}
37+
// CHECK: -----together-----
38+
// CHECK: s1.o-[[S1_HASH]]
39+
// CHECK: --------
40+
// CHECK: out-file: REMAPPED_OUT_DIR{{/|\\}}s1.o
41+
// CHECK: s2.o-[[S2_HASH]]
42+
// CHECK: --------
43+
// CHECK: out-file: REMAPPED_OUT_DIR{{/|\\}}s2.o
44+
45+
// CHECK: -----together-relative-----
46+
// CHECK: s1.o-[[S1_HASH]]
47+
// CHECK: --------
48+
// CHECK: out-file: REMAPPED_OUT_DIR{{/|\\}}s1.o
49+
// CHECK: s2.o-[[S2_HASH]]
3650
// CHECK: --------
3751
// CHECK: out-file: REMAPPED_OUT_DIR{{/|\\}}s2.o
38-
// CHECK: DEPEND START
39-
// CHECK: Unit | system | {{.*}}Swift.swiftmodule
40-
// CHECK: DEPEND END

‎test/Index/Store/unit-one-sourcefile-remapped.swift‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
1-
// RUN: rm -rf %t
1+
// RUN: %empty-directory(%t)
22
// RUN: mkdir -p %t/BuildRoot && cd %t/BuildRoot
3+
4+
// Check paths are mapped
35
// RUN: %target-swift-frontend -index-store-path %t/idx -file-prefix-map %S=REMAPPED_SRC_ROOT -file-prefix-map %t=REMAPPED_OUT_DIR %s -o %t/file1.o -typecheck
4-
// RUN: c-index-test core -print-unit %t/idx | %FileCheck %s --dump-input-filter all
6+
// RUN: c-index-test core -print-unit %t/idx > %t/units.out
7+
8+
// Check round-trip remapping to make sure they're converted back to the local paths
9+
// RUN: c-index-test core -print-unit %t/idx -index-store-prefix-map REMAPPED_SRC_ROOT=%S -index-store-prefix-map REMAPPED_OUT_DIR=%t >> %t/units.out
10+
11+
// RUN: %FileCheck %s --dump-input-filter all < %t/units.out
512

6-
// CHECK: file1.o-1AYKXZF3HH50A
13+
// CHECK: file1.o-[[UNIT_HASH:.*$]]
714
// CHECK: --------
815
// CHECK: main-path: REMAPPED_SRC_ROOT{{/|\\}}unit-one-sourcefile-remapped.swift
916
// CHECK: work-dir: REMAPPED_OUT_DIR{{/|\\}}BuildRoot
@@ -12,11 +19,9 @@
1219
// CHECK: Unit | system | Swift | {{BUILD_DIR|.*lib\\swift\\windows}}{{.*}}Swift.swiftmodule
1320
// CHECK: DEPEND END
1421

22+
// CHECK: file1.o-[[UNIT_HASH]]
23+
// CHECK: --------
24+
// CHECK: main-path: SOURCE_DIR{{/|\\}}test{{/|\\}}Index{{/|\\}}Store{{/|\\}}unit-one-sourcefile-remapped.swift
25+
// CHECK-NOT: work-dir: REMAPPED_OUT_DIR
1526

16-
// Check round-trip remapping to make sure they're converted back to the local paths.
17-
// RUN: c-index-test core -print-unit %t/idx -index-store-prefix-map REMAPPED_SRC_ROOT=%S -index-store-prefix-map REMAPPED_OUT_DIR=%t | %FileCheck %s -check-prefix=ROUNDTRIP --dump-input-filter all
18-
19-
// ROUNDTRIP: file1.o-1AYKXZF3HH50A
20-
// ROUNDTRIP: --------
21-
// ROUNDTRIP: main-path: SOURCE_DIR{{/|\\}}test{{/|\\}}Index{{/|\\}}Store{{/|\\}}unit-one-sourcefile-remapped.swift
22-
// ROUNDTRIP-NOT: work-dir: REMAPPED_OUT_DIR
27+
// CHECK-NOT: file1.o-[[UNIT_HASH]]

‎test/Index/Store/unit-pcm-dependency-remapped.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ func test() {
5959
// FILE1: Record | user | {{.*}}ClangModuleB.h | ClangModuleB.h-
6060
// FILE1: DEPEND END
6161

62-
// FILE1: s1.o-{{2LQAU7D8TZHZ8|2RHC8ZJFDYDW4}}
62+
// FILE1: s1.o-
6363
// FILE1: --------
6464
// FILE1: has-main: 1
6565
// FILE1-ABSOLUTE: main-path: REMAPPED_SRC_DIR{{.*}}unit-pcm-dependency-remapped.swift
@@ -81,7 +81,7 @@ func test() {
8181

8282
// FILE2-NOT: main.swiftmodule-
8383

84-
// FILE2: s2.o-{{2OIL2LG8UULK6|15MCL6ZLKZKNL}}
84+
// FILE2: s2.o-
8585
// FILE2: --------
8686
// FILE2: has-main: 1
8787
// FILE2: main-path: REMAPPED_OUT_DIR{{.*}}s2.swift

‎test/Index/Store/unit-swiftmodule-dependency.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func test() {
2727
// CHECK: [[MODB:SwiftModuleB.swiftmodule-[A-Z0-9]*]]
2828
// CHECK: --------
2929
// CHECK: has-main: 1
30-
// CHECK: out-file: {{.*}}/SwiftModuleB.swiftmodule
30+
// CHECK: out-file: {{.*}}{{/|\\}}SwiftModuleB.swiftmodule
3131
// CHECK: DEPEND START
3232
// CHECK: Unit | system | Swift | {{.*}}{{/|\\}}Swift.swiftmodule
3333
// CHECK: Unit | user | SwiftModuleA | {{.*}}{{/|\\}}SwiftModuleA.swiftmodule

0 commit comments

Comments
 (0)