destructor, sync option

[rwebb]

I suggest you add and option for the destructor to sync. Something like m.destructor_sync(true);

I believe there are use cases when data is being updated, where forgetting to sync could be a costly and hard to find error. With a optional destructor_sync (defaulting to false) you can create a wrapper that makes RW maps sync on destruction. Seems easy to implement and more flexible.

[vimpunk]

@rwebb Thanks for your input. This is something I've been pondering as well and in retrospect it seems like it was a bad idea to forgo syncing in the destructor. Even though it's not an API change, I'm a bit hesitant to make this change now. What do you think? ~Yur solution of introducing a configuration option could also work, though I'd prefer to keep the API simple.

[rwebb]

The required API and semantics would remain the same with my proposal. Having a simple setting method of m.destructor_sync(true) doesn't seem like a serious API burden.

In my opinion there are use cases for both sync-on-destruction and no-sync-on-destruction. So it just comes down to which is the default and do you have an optional constructor argument or a separate method. A separate setting method is more flexible. Whether to have ON or OFF as the default is debatable (you seem to have already chosen that though).

[vimpunk]

@rwebb Ok, you've convinced me. Would you be willing to send a PR? Also, what is your opinion on the most sensible default behavior?

[rwebb]

I don't have the change implemented. I agree with you memmaps are great, and I'm looking forward to having sync on destruction be an option.

For default, I would go with auto-sync for RW maps, but as you have non-auto-sync now you may not want to change it. Your call.

[vimpunk]

Thank you for your input, I'll land the changes in a bit.

[vimpunk]

@rwebb I've implemented the changes in this branch, but I chose to implement the destructor policy as an additional optional non-type template parameter. I wanted to avoid increasing the object's size but after implementing this I realized I already have a bool member that will be padded to 64-bits anyway, so it doesn't matter if I add another one... However, it does have an advantage in that it's perhaps more explicit and less error-prone if no-sync in the destructor is really required, because by creating a type alias (using nosync_mmap_sink = mio::basic_mmap_sink<char, mio::dtor_policy::no_sync>;) and only using that type going forward, no extra method would have to be called and the no-sync behavior is always ensured. But I'm unsure, so input is appreciated.