provide Base.crc32c checksum and use it in checking cache staleness

[stevengj]

This fixes #17845: to reduce the chance of false positives when checking cache staleness, it also checks a CRC-32c checksum of the file if the timestamp doesn't match. (That is, the file is considered stale if timestamp mismatch && checksum mismatch).

The performance impact seems to be negligible. In the common case where the timestamps match, the checksum is never computed at all, and in any case the computation of a checksum seems to be orders of magnitude faster than the time to load the cache file.

I used Mark Adler's (@madler) BSD-licensed zlib-licensed CRC-32c implementation that he posted on StackOverflow, which includes both software and hardware-accelerated versions (the latter for SSE4.2 hardware). Adler seems to be someone who knows what he is doing with checksum algorithms, so I was happy to see he posted usable code. His code seems to be pretty fast (well over an order of magnitude faster than @andrewcooke's implementation in CRC.jl, which Andrew said is comparable to zlib; see also andrewcooke/CRC.jl#5). @baruch did some benchmarking and he found that Adler's implementation was one of the fastest. I don't see much point in re-implementing it in Julia.

Because of the existence of hardware support, CRC-32c seems like the most sensible choice of checksum algorithm, so I didn't really look at alternatives.

It also seems like a good idea to export the crc32c checksum function, since this is pretty generally useful functionality to have, but I wanted to get feedback first.

[tkelman]

should be listed in the license file and exception list in contrib/add_license_to_files.jl.

[tkelman]

should fall back to the software version for _COMPILER_MICROSOFT_

[stevengj]

Done.

[stevengj]

Fixed the license file and updated the tests.

[tkelman]

you might need a bit more than 2 seconds to ensure the date changes

(not serious)

[stevengj]

We could use __cpuid and _mm_crc32_u64 with MS compilers, rather than inline asm.

[tkelman]

HW_CRC ?

[stevengj]

(Speaking of future optimizations, ARMv8 also has a CRC32CX instruction for CRC-32c of 8 bytes.)

[stevengj]

Yay, tests are green.

[Keno]

Missing newline and -?

[stevengj]

No, the line has a newline and a -. Maybe the diff format is confusing?

[Keno]

Yes, sorry, I'm gonna go clean my glasses.

[stevengj]

@StefanKarpinski, @vtjnash, any comment?

[vtjnash]

I'm still against using a checksum to validate files. Remember too that with precompile, you will pay this cost m*n^2 times, so even a small cost quickly adds up and requires moving many megabytes of extra data which may not be local (the precompile .ji files are all supposed to be local)

[stevengj]

@vtjnash, the files whose timestamps don't match but whose checksums match will typically be small deps.jl files re-generated by Pkg.build. There is no way that the time to checksum those, even if they are on a remote server (with reasonable speed), will be slower than the time to load a module from the cache.

The number of times that this occurs is irrelevant, because in the cases where you run the same checksum a large number of times you are also loading the module from a cache a large number of times.

[vtjnash]

We can easily fix that bug in BinDeps rather than working around it here.

I don't see any reason loading a module should be slower than doing CRC over the sources. In fact, I think that loading the module should be less work.

I don't find it terribly convincing that this only affects a small number of cases, for me that's also a negative.

Another very strong negative, IMO, is that doing touch or save in my editor won't trigger a rebuild. With this change, I now need to know where to find a hidden cache file and delete it if something goes wonky. That violates the goal of the cache being fully transparent.

[tkelman]

Changing the timestamp without changing the content shouldn't trigger a rebuild. Timestamps aren't a great proxy for matching content. This will make .ji files more portable and easier to deploy as part of prebuilding Julia along with sets of packages.

[vtjnash]

Timestamps aren't simply a proxy for content, they are part of the content, and one that is easy for the user to influence. In that regard, I could perhaps accept stale = (same-file-path | same-crc) && same-time-stamp. But if you're going to make a claim to the contrary, you at least need to give some supporting rationale

Deployment is a different process with different requirements. I don't see any reason to couple it to the behavior of precompile.

[tkelman]

if you're going to make a claim to the contrary, you at least need to give some supporting rationale

Because the timestamp of a Julia file doesn't change the end-result behavior of the code contained in it whatsoever?

[vtjnash]

Hah. That assumes that the compiler is pure with respect to the universe, which is nonsense. It's not even best effort; I would give it a grade of: "tries to be fairly predictable if everything goes well". But fortunately a changed timestamp is a strong indicator that the user did intend to change something, so we can mostly recover from the inaccuracy of the rest of the system.

[tkelman]

include doesn't change behavior depending on the timestamp of the file, except as an implementation detail of the current staleness check. If we're arguing about "strong indicator that the user did intend to change something" then we're trying to read minds, and so far you're the only person arguing on the "timestamps should matter" side vs multiple people who've asked for them not to be the sole criterion - #17845 (comment).

[vtjnash]

BinDeps set the "this dependency has changed and needs to be rebuilt" flag. It's not the fault of base that we honor this flag, nor the fault of the original author (since the capability didn't exist at the time). But seriously, fix BinDeps.

I'm not trying to read minds, because I don't have to. I can simply read the filesystem metadata that has been the standard for build systems, noting that the frequent failure there is that it is not strict enough, and arguing that we shouldn't react by making our heuristics weaker. Nobody ever wished that it might not be possible to get make to recompile a C file after changing it. Instead, you re-touch the source file and let the build system take care of propagating that to the result.

include also rebuilds the whole cache. You've lost me on how this is an argument for not rebuilding the cache.

[tkelman]

#17845 (comment) wasn't about BinDeps.

You're equating filesystem metadata with "indicator that the user did intend to change something," when they're often not connected at all.

Nobody ever wished that it might not be possible to get make to recompile a C file after changing it.

The existence of ccache disagrees with you.

[vtjnash]

when they're often not connected at all.

Rebuilding a false positive is fine as long as it doesn't get stuck repeatedly attempting to the update. It is not acceptable for a build system to have "not often" false negatives which have no easy recourse for the user to fix them.

The existence of ccache disagrees with you.

not at all, it agrees with me perfectly. First of all, ccache is a compiler, not a make dependency system. Also, it's a bug if ccache output isn't perfectly captured by the input (https://ccache.samba.org/manual.html#_bugs), and it includes rigorous checks that the inputs are the same, such as requiring that the filenames are exactly the same and parsing the source to prove it doesn't contain any content that could result in a detectable difference in output.

I actually intend to write a ccache-like too for Julia, although designed around Julia's semantics so it will be completely different.

[stevengj]

Okay, I'm convinced.

[StefanKarpinski]

I don't see any reason loading a module should be slower than doing CRC over the sources. In fact, I think that loading the module should be less work.

I have to call b.s. on this statement. Are you really claiming that reading, parsing and compiling a Julia file will take less time than computing a CRC on the file?

[vtjnash]

no, I'm claiming that doing a crc on the source would add a significant percentage to the time to load the precompile file, so it would add a bit of unpredictable uncertainty to the load time. In other-words, I'm committing to making precompile loading much faster :)

[vtjnash]

fwiw, please don't delete this branch, as having CRC code is likely to be useful (for example, we could be using it to validate that the .ji files aren't truncated or corrupted.)

[stevengj]

@vtjnash, yes, that was my thought.

[stevengj]

Maybe I should open another PR with just the CRC?

[vtjnash]

sgtm. If you didn't, I was planning on doing it myself. Probably not a huge cost, but I would prefer – flr the sake of startup time – to either make the table data fully static, or lazy initialize it (using uv_once), with preference for the former as it allows the kernel to share these data pages between processes.