Skip to content

Commit b2256da

Browse files
LperlindLperlind
committed
cached.cpp: Improve internal cached robustness
Do not memory map files; Windows cannot write to a file that is memory mapped. Write cache after llvm building. This ensures the cache won't have a false positive if llvm fails.
1 parent 7c1922b commit b2256da

File tree

2 files changed

+75
-55
lines changed

2 files changed

+75
-55
lines changed

‎src/cached.cpp‎

Lines changed: 61 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -187,10 +187,7 @@ gb_internal bool try_copy_executable_from_cache(void) {
187187
extern char **environ;
188188
#endif
189189

190-
// returns false if different, true if it is the same
191-
gb_internal bool try_cached_build(Checker *c, Array<String> const &args) {
192-
TEMPORARY_ALLOCATOR_GUARD();
193-
190+
Array<String> cache_gather_files(Checker *c) {
194191
Parser *p = c->parser;
195192

196193
auto files = array_make<String>(heap_allocator());
@@ -222,29 +219,11 @@ gb_internal bool try_cached_build(Checker *c, Array<String> const &args) {
222219

223220
array_sort(files, string_cmp);
224221

225-
u64 crc = 0;
226-
for (String const &path : files) {
227-
crc = crc64_with_seed(path.text, path.len, crc);
228-
}
229-
230-
String base_cache_dir = build_context.build_paths[BuildPath_Output].basename;
231-
base_cache_dir = concatenate_strings(permanent_allocator(), base_cache_dir, str_lit("/.odin-cache"));
232-
(void)check_if_exists_directory_otherwise_create(base_cache_dir);
233-
234-
gbString crc_str = gb_string_make_reserve(permanent_allocator(), 16);
235-
crc_str = gb_string_append_fmt(crc_str, "%016llx", crc);
236-
String cache_dir = concatenate3_strings(permanent_allocator(), base_cache_dir, str_lit("/"), make_string_c(crc_str));
237-
String files_path = concatenate3_strings(permanent_allocator(), cache_dir, str_lit("/"), str_lit("files.manifest"));
238-
String args_path = concatenate3_strings(permanent_allocator(), cache_dir, str_lit("/"), str_lit("args.manifest"));
239-
String env_path = concatenate3_strings(permanent_allocator(), cache_dir, str_lit("/"), str_lit("env.manifest"));
240-
241-
build_context.build_cache_data.cache_dir = cache_dir;
242-
build_context.build_cache_data.files_path = files_path;
243-
build_context.build_cache_data.args_path = args_path;
244-
build_context.build_cache_data.env_path = env_path;
222+
return files;
223+
}
245224

225+
Array<String> cache_gather_envs() {
246226
auto envs = array_make<String>(heap_allocator());
247-
defer (array_free(&envs));
248227
{
249228
#if defined(GB_SYSTEM_WINDOWS)
250229
wchar_t *strings = GetEnvironmentStringsW();
@@ -275,19 +254,50 @@ gb_internal bool try_cached_build(Checker *c, Array<String> const &args) {
275254
#endif
276255
}
277256
array_sort(envs, string_cmp);
257+
return envs;
258+
}
259+
260+
// returns false if different, true if it is the same
261+
gb_internal bool try_cached_build(Checker *c, Array<String> const &args) {
262+
TEMPORARY_ALLOCATOR_GUARD();
263+
264+
auto files = cache_gather_files(c);
265+
auto envs = cache_gather_envs();
266+
defer (array_free(&envs));
267+
268+
u64 crc = 0;
269+
for (String const &path : files) {
270+
crc = crc64_with_seed(path.text, path.len, crc);
271+
}
272+
273+
String base_cache_dir = build_context.build_paths[BuildPath_Output].basename;
274+
base_cache_dir = concatenate_strings(permanent_allocator(), base_cache_dir, str_lit("/.odin-cache"));
275+
(void)check_if_exists_directory_otherwise_create(base_cache_dir);
276+
277+
gbString crc_str = gb_string_make_reserve(permanent_allocator(), 16);
278+
crc_str = gb_string_append_fmt(crc_str, "%016llx", crc);
279+
String cache_dir = concatenate3_strings(permanent_allocator(), base_cache_dir, str_lit("/"), make_string_c(crc_str));
280+
String files_path = concatenate3_strings(permanent_allocator(), cache_dir, str_lit("/"), str_lit("files.manifest"));
281+
String args_path = concatenate3_strings(permanent_allocator(), cache_dir, str_lit("/"), str_lit("args.manifest"));
282+
String env_path = concatenate3_strings(permanent_allocator(), cache_dir, str_lit("/"), str_lit("env.manifest"));
283+
284+
build_context.build_cache_data.cache_dir = cache_dir;
285+
build_context.build_cache_data.files_path = files_path;
286+
build_context.build_cache_data.args_path = args_path;
287+
build_context.build_cache_data.env_path = env_path;
278288

279289
if (check_if_exists_directory_otherwise_create(cache_dir)) {
280-
goto write_cache;
290+
return false;
281291
}
282292

283293
if (check_if_exists_file_otherwise_create(files_path)) {
284-
goto write_cache;
294+
return false;
285295
}
286296
if (check_if_exists_file_otherwise_create(args_path)) {
287-
goto write_cache;
297+
return false;
288298
}
289299
if (check_if_exists_file_otherwise_create(env_path)) {
290-
goto write_cache;
300+
return false;
291301
}
292302

293303
{
@@ -297,7 +307,7 @@ gb_internal bool try_cached_build(Checker *c, Array<String> const &args) {
297307
LoadedFileError file_err = load_file_32(
298308
alloc_cstring(temporary_allocator(), files_path),
299309
&loaded_file,
300-
false
310+
true
301311
);
302312
if (file_err > LoadedFile_Empty) {
303313
return false;
@@ -315,7 +325,7 @@ gb_internal bool try_cached_build(Checker *c, Array<String> const &args) {
315325
}
316326
isize sep = string_index_byte(line, ' ');
317327
if (sep < 0) {
318-
goto write_cache;
328+
return false;
319329
}
320330

321331
String timestamp_str = substring(line, 0, sep);
@@ -325,21 +335,21 @@ gb_internal bool try_cached_build(Checker *c, Array<String> const &args) {
325335
path_str = string_trim_whitespace(path_str);
326336

327337
if (file_count >= files.count) {
328-
goto write_cache;
338+
return false;
329339
}
330340
if (files[file_count] != path_str) {
331-
goto write_cache;
341+
return false;
332342
}
333343

334344
u64 timestamp = exact_value_to_u64(exact_value_integer_from_string(timestamp_str));
335345
gbFileTime last_write_time = gb_file_last_write_time(alloc_cstring(temporary_allocator(), path_str));
336346
if (last_write_time != timestamp) {
337-
goto write_cache;
347+
return false;
338348
}
339349
}
340350

341351
if (file_count != files.count) {
342-
goto write_cache;
352+
return false;
343353
}
344354
}
345355
{
@@ -348,7 +358,7 @@ gb_internal bool try_cached_build(Checker *c, Array<String> const &args) {
348358
LoadedFileError file_err = load_file_32(
349359
alloc_cstring(temporary_allocator(), args_path),
350360
&loaded_file,
351-
false
361+
true
352362
);
353363
if (file_err > LoadedFile_Empty) {
354364
return false;
@@ -366,11 +376,11 @@ gb_internal bool try_cached_build(Checker *c, Array<String> const &args) {
366376
break;
367377
}
368378
if (args_count >= args.count) {
369-
goto write_cache;
379+
return false;
370380
}
371381

372382
if (line != args[args_count]) {
373-
goto write_cache;
383+
return false;
374384
}
375385
}
376386
}
@@ -380,7 +390,7 @@ gb_internal bool try_cached_build(Checker *c, Array<String> const &args) {
380390
LoadedFileError file_err = load_file_32(
381391
alloc_cstring(temporary_allocator(), env_path),
382392
&loaded_file,
383-
false
393+
true
384394
);
385395
if (file_err > LoadedFile_Empty) {
386396
return false;
@@ -398,20 +408,26 @@ gb_internal bool try_cached_build(Checker *c, Array<String> const &args) {
398408
break;
399409
}
400410
if (env_count >= envs.count) {
401-
goto write_cache;
411+
return false;
402412
}
403413

404414
if (line != envs[env_count]) {
405-
goto write_cache;
415+
return false;
406416
}
407417
}
408418
}
409419

410420
return try_copy_executable_from_cache();
421+
}
422+
423+
void write_cached_build(Checker *c, Array<String> const &args) {
424+
auto files = cache_gather_files(c);
425+
defer (array_free(&files));
426+
auto envs = cache_gather_envs();
427+
defer (array_free(&envs));
411428

412-
write_cache:;
413429
{
414-
char const *path_c = alloc_cstring(temporary_allocator(), files_path);
430+
char const *path_c = alloc_cstring(temporary_allocator(), build_context.build_cache_data.files_path);
415431
gb_file_remove(path_c);
416432

417433
debugf("Cache: updating %s\n", path_c);
@@ -426,7 +442,7 @@ write_cache:;
426442
}
427443
}
428444
{
429-
char const *path_c = alloc_cstring(temporary_allocator(), args_path);
445+
char const *path_c = alloc_cstring(temporary_allocator(), build_context.build_cache_data.args_path);
430446
gb_file_remove(path_c);
431447

432448
debugf("Cache: updating %s\n", path_c);
@@ -441,7 +457,7 @@ write_cache:;
441457
}
442458
}
443459
{
444-
char const *path_c = alloc_cstring(temporary_allocator(), env_path);
460+
char const *path_c = alloc_cstring(temporary_allocator(), build_context.build_cache_data.env_path);
445461
gb_file_remove(path_c);
446462

447463
debugf("Cache: updating %s\n", path_c);
@@ -454,8 +470,5 @@ write_cache:;
454470
gb_fprintf(&f, "%.*s\n", LIT(env));
455471
}
456472
}
457-
458-
459-
return false;
460473
}
461474

‎src/main.cpp‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3391,6 +3391,7 @@ int main(int arg_count, char const **arg_ptr) {
33913391

33923392
Parser *parser = gb_alloc_item(permanent_allocator(), Parser);
33933393
Checker *checker = gb_alloc_item(permanent_allocator(), Checker);
3394+
bool failed_to_cache_parsing = false;
33943395

33953396
MAIN_TIME_SECTION("parse files");
33963397

@@ -3480,6 +3481,7 @@ int main(int arg_count, char const **arg_ptr) {
34803481
if (try_cached_build(checker, args)) {
34813482
goto end_of_code_gen;
34823483
}
3484+
failed_to_cache_parsing = true;
34833485
}
34843486

34853487
#if ALLOW_TILDE
@@ -3545,18 +3547,23 @@ int main(int arg_count, char const **arg_ptr) {
35453547

35463548
end_of_code_gen:;
35473549

3548-
if (build_context.show_timings) {
3549-
show_timings(checker, &global_timings);
3550-
}
3551-
35523550
if (build_context.export_dependencies_format != DependenciesExportUnspecified) {
35533551
export_dependencies(checker);
35543552
}
35553553

3554+
if (build_context.cached) {
3555+
MAIN_TIME_SECTION("write cached build");
3556+
if (!build_context.build_cache_data.copy_already_done) {
3557+
try_copy_executable_to_cache();
3558+
}
3559+
3560+
if (failed_to_cache_parsing) {
3561+
write_cached_build(checker, args);
3562+
}
3563+
}
35563564

3557-
if (!build_context.build_cache_data.copy_already_done &&
3558-
build_context.cached) {
3559-
try_copy_executable_to_cache();
3565+
if (build_context.show_timings) {
3566+
show_timings(checker, &global_timings);
35603567
}
35613568

35623569
if (run_output) {

0 commit comments

Comments
 (0)