4 ms·
"In hindsight it's difficult to justify using mmap over pread" This needs a stronger justification. mmap allows reading and writing large data structures witho
by eeZah7Ux 8y ago
"In hindsight it's difficult to justify using mmap over pread"
This needs a stronger justification. mmap allows reading and writing large data structures without copying, which can be a huge benefit depending on the use case.
- maxlybbert 8y agoI think it’s reasonable to say “it’s difficult to justify using mmap when other programs are expected to manipulate, and potentially even delete, the file while you are working with it.” But, honestly, that case will always be hard to handle, and mmap doesn’t make it any worse.
- nneonneo 8y agoI think the problem here is that mmap does make it worse - with (p)read, if you get an errno, you flag an error. with mmap, you have to handle a signal and jump back to an appropriate point in the code (and you can't do this in a cross-platform way), and then flag an error. Obviously, what the upper levels do with the error still has to be worked out, but you can hardly argue that sigsetjmp/siglongjmp + signal handlers + Windows SEH handlers is no worse than "ret = read(...); if (ret < 0) ERROR;"
- maxlybbert 8y agoIf the file changes out from under you, all you can do is reread the file. mmap gives you a pointer that you can read from, but you still have to loop through the file’s bytes. I’m pretty sure most programmers are capable of writing a signal handler that sets a flag (volatile sig_atomic_t) for “parsing failed,” and a loop that checks that flag in addition to checking whether the loop is finished for other reasons. Signal handling doesn’t have to be complicated.
- nneonneo 8y agoIt’s not quite as simple as setting a flag - if the signal is SIGBUS/SIGSEGV then returning from the handler normally will retry the faulting instruction. So, you either have to map dummy memory into place so the fault doesn’t reoccur, or use sigsetjmp/siglongjmp to jump out to a previously configured exception handler (as Sublime Text did here. I think you’ve mixed up synchronous signals - SEGV, BUS, ABRT - with asynchronous signals like QUIT, INT, USR1. Asynchronous signals can be handled easily with a flag and a loop as you mentioned; synchronous signals are much trickier and much more complicated.
- ben-schaaf 8y agoUsing pread would have been more work from the start but provides a more robust solution and doesn't have problems on Windows. I would argue that the incrementaly built mmap based solution is strictly worse, thus difficult to justify doing again.
- eeZah7Ux 8y ago"strictly worse" despite no-copy memory access? Again, this needs proof.
- ben-schaaf 8y agoStrictly worse because it requires more work to maintain and write new code, there's no guarantee we haven't missed any access points in our codebase so it is less robust, still locks files while in use on Windows and requires maintaining patches to Breakpad. Performance is not an issue here, the program working correctly is, and doing so in the long run is strictly worse than mmap.
- superlopuh 8y agoDo you have a list of these strictly-better things to do and a plan on when to implement them? Do you let them pile up? Mostly asking about your decision-making process.
- ben-schaaf 8y agoI'm not sure what you mean. The blog post discusses solutions to all the problems I listed. They wouldn't have been problems using pread, which has a straight forward implementation.
- iainmerrick 8y agoThe original post quantifies it. Around 50% better performance for the mmap version. They are saying that if they somehow knew up front what the performance gains would be, and what the cost in bugs and complexity would be, they wouldn’t have used mmap at all.
- quotemstr 8y ago> without copying While it's true that memory bandwidth is sometimes a limiting factor in performance, I've found that much more frequently people overestimate the cost of memory operations and don't check their estimates against benchmarks.
- eeZah7Ux 8y agoIndeed. It depends on the use case.
- eeZah7Ux 8y ago...and why the downvotes?
- eeZah7Ux 8y ago...keep going...