Conversation
Signed-off-by: Rudi Grinberg <[email protected]>
|
@Khady does this PR make a meaningful difference? |
|
@voodoos Independently of this pr: the preprocessing handling in the merlin code seems wrong to me. It seems like we don't handle |
Do not do the same work for every module in a library Signed-off-by: Rudi Grinberg <[email protected]>
Signed-off-by: Rudi Grinberg <[email protected]>
rgrinberg
force-pushed
the
speed-up-merlin
branch
from
d6c2de9 to
965dfe5
Compare
Signed-off-by: Rudi Grinberg <[email protected]>
Signed-off-by: Rudi Grinberg <[email protected]>
| ++ Pp.newline | ||
| ++ Pp.vbox | ||
| (Pp.concat_map modules ~f:(fun m -> | ||
| Pp.text (Module_name.to_string m))) |
There was a problem hiding this comment.
This is used for debugging and testing only, and it feels less useful to have a module's name printed after its configuration (and it brings lot of small changes to the tests). Is there a specific reason why you chose to switch them ?
There was a problem hiding this comment.
I just thought it made the tests neater. Now we aren't printing the same configuration over and over.
There was a problem hiding this comment.
I agree that merging identical configuration is better, I was just complaining about the module names being printed after and not before the configuration.
However I had to re-duplicate for Per_module pp configs #4092.
|
These changes looks good to me.
That's probably true, I will have a look today. |
|
@rgrinberg I haven't checked if the merlin files are the same as before, but the the noop warm build went from 7.6s with 2.7.1 to 4.6s with this PR. Part of the gain is from #4064. But I think that 4.6s is at least a whole second faster than the numbers I got for #4064. |
Yes, I didn't expect much more than that. A fresh profile with this PR would be helpful. This RP removes a bunch of unnecessary maps and sets operations that overlap with other parts of the code, so the profile should look somewhat different now. |
|
I will provide one tomorrow |
|
Closed in favor of #4092 |
3 participants
We're doing the same work for every single module in a library. This PR makes it
do it only once.