search

janestreet / memtrace

Streaming client for OCaml's Memprof
MIT License
65 stars 13 forks source link

Fix encoding of large 32-bit integers #4

Closed stedolan closed 3 years ago

stedolan commented 3 years ago

See #1

dra27 commented 3 years ago

This is better - does it need to round-trip correctly across platform? I'd just arrived at a similar fix, but hadn't yet convinced myself of that (I think the answer is no it doesn't need to?!)

stedolan commented 3 years ago

Yeah, even with this patch it remains the case that if you take a trace on a 64-bit machine that does large allocations (>16GB, I think), and process it on a 32-bit machine, then some numbers will be truncated. I think we can probably live with that?

dra27 commented 3 years ago

I think it's fine, isn't it - a 32-bit trace will always be read back correctly on a 64-bit system, which is the important direction.

The hardening I was considering would be on a 64-bit system only to use Int32 for 0 < x < 0x3fffffff - i.e. only use Int32 for OCaml values which round-trip the same way on both 64 and 32-bit systems. It would then be simple enough to have a check in the Int64 branch of get_vint_big to detect truncation. However, I think "use a 64-bit viewer" is a not unreasonable approach :)