Replace shm.map with separate create() and open()

[wingo]

Currently shm.map() will create the file and any containing directories
if it does not exist. This is not so great, so I split it into separate
open() and create() functions, adapt all callers, and remove map() from
the public interface of the shm module.

[mention-bot]

By analyzing the blame information on this pull request, we identified @lukego, @eugeneia and @llelf to be potential reviewers

[lukego]

"hierarchical", "brevity", "artefact", "bureaucracy"... damn these words :)

[eugeneia]

This is not so great

I fail to see why. Can you elaborate? As far as I am concerned this changes the API around file system semantics that are not relevant to the API. E.g. before you could first map an object read only, then another process could map that object, write to it, and it would work. Now you need to make sure that you open an object only after it was created by another process. Correct me if I am wrong.

[lukego]

I should consider whether this helps solve the problem with #767.

[wingo]

E.g. before you could first map an object read only, then another process could map that object, write to it, and it would work. Now you need to make sure that you open an object only after it was created by another process. Correct me if I am wrong.

This is correct. It could be that we always want to be using create(). It seems wrong to me though in some deep way :) Like, a process that is just reading data doesn't want to create it if it doesn't exist. The fact that a counter doesn't exist could be interesting information that would be perturbed by the reader. For example if there is counter data that has some kind of complicated interior structure I would want to be sure to initialize it correctly; dunno.

Concretely snabb top will show latency information if the latency histogram is there, and not otherwise.

Perhaps this is worrying too much though; other thoughts are welcome.

[wingo]

Added on a couple commits I found while porting the histogram to use this.

[wingo]

I would like to have the histogram stuff off my plate by the end of the week, so review here would be appreciated @eugeneia :)

[eugeneia]

@wingo Sorry, somehow I missed the previous comments. I gave this a lot of thought and while I generally feel bad about extending an API based on gut feeling, I do share the same gut feeling and feel that this new API is more hygienic.

What is missing though is the adaption of snabb top. As it is now this PR would break snabb top as far as I can tell?

[wingo]

Thanks for the review. I am not sure how snabb top would break; it does seem to work for me in #795, but I will check tomorrow to make sure.

[eugeneia]

It works because #795 updates snabb top to use open instead of map I presume. Please keep PRs atomic in the future (if there was a CI test for snabb top this would not have passed).

[wingo]

Hmm, OK. I don't see the change you refer to in #795; would you mind pointing it out? I do try to keep these things atomic but I occasionally make mistakes :)

[eugeneia]

@wingo Oops, my bad. Somehow I was absolutely sure that snabb top uses shm.map (while it doesn't).

[wingo]

Thanks very much for the review and merge; cheers :)