Fix Merlin per-module pp flags

[voodoos]

Recent changes made merlin configuration files distinct for each module in a lib of exe. However we were not taking advantage on that for preprocessing, this PR correct that.

This should fix issues: #2596 #1212 and #3409

@rgrinberg this PR is based on #4091. I shared the "processing" work for every module using the same pp specification. However the shared Per_item data-structure does not play well with Build.With_target.t and is not easily translatable to a (key list, 'a) list) while preserving sharing. I decided to duplicate these once-processed pp flags for each module, does that seem reasonable to you ?

[voodoos]

The only option to keep sharing I see would be to add a function to Per_element because we need access to the array used in the implementation. The function could be something like that:

let map_with_targets { map; values } ~f =
  let open Build.With_targets.O in
  let l = Array.to_list values in
  let+ new_values = List.map l ~f |> Build.With_targets.all in
  {
    map;
    values = Array.of_list new_values
  }

[rgrinberg]

I shared the "processing" work for every module using the same pp specification. However the shared Per_item data-structure does not play well with Build.With_target.t and is not easily translatable to a (key list, 'a) list)

That is a problem for all of our containers. They require this projection and injection to lists to go from 'a Build.t container to 'a container Build.t.

I decided to duplicate these once-processed pp flags for each module, does that seem reasonable to you ?

Sadly, I don't think this is good enough.

The only option to keep sharing I see would be to add a function to Per_element because we need access to the array used in the implementation. The function could be something like that:

That sounds much better to me.

By the way, we should maintain the Per_item representation when marshalling. We don't want to write bloated marshalled files unnecessarily, the reader could do the lookup inside the Per_item for the module of interest just as easily

[voodoos]

@rgrinberg : should be better like this. Sharing is preserved until the marshaled config is read and pp'ed by the merlin-server.

However the debug printing is duplicating again because the data-structure used in Per_item does not allow for the easy recovery of all members of a same equivalence class. But this is only for the debug print anyway.

[Khady]

A fresh profile with this PR would be helpful.

• f51644a (this PR) trace.zip
• e7a0e93 (previous PR) trace.zip

[rgrinberg]

Looks good apart from some minor things.

[rgrinberg]

Thanks @Khady . Could you also give a perf report of the last commit here.

I can confirm that all merlin related allocation is now absent from the trace. Apart from the things I mentioned earlier, I don't have any ideas on what else we can optimize. Perhaps @aalekseyev will find something though.

[aalekseyev]

In the profile I couldn't see any obvious single thing to work on.
A lot of it seems to be overhead of Fiber and Memo, though. For example add_dep_from_caller seems to account for ~15% of all allocations, which is not what I would expect.

[rgrinberg]

@voodoos Ready to merge!

Actually this PR also fixes a bug along the way: directories with mix preprocessors specifications are now handled correctly. You should mention that in the CHANGES along with the appropriate bug numbers for it.

[Khady]

Could you also give a perf report of the last commit here.

Here it is perf.zip