Migrate away from loupe #1663
Migrate away from loupe #1663
Conversation
This essentially reverts #1041
There was a problem hiding this comment.
LGTM. Perhaps adding a factor to increase the estimated size is a good idea. Here on in a follow-up PR.
The factor can be based on the numbers from here: #959 (comment) (50 / 60 % increase or so).
Anyway, if all the modules are being underestimated by more or less the same amount, this is not a big deal / probably not needed.
| @@ -125,17 +129,20 @@ impl FileSystemCache { | |||
| } | |||
|
|
|||
| /// Stores a serialized module to the file system. Returns the size of the serialized module. | |||
| pub fn store(&mut self, checksum: &Checksum, module: &Module) -> VmResult<()> { | |||
| /// The serialized module size is a good approximation (~100.06 %) of the in-memory module size. | |||
There was a problem hiding this comment.
This percentage (~100.06%) was derived by comparing the serialised size to the size of the memory reported by massif. As such, is not considering mmaped memory, used in the module's storage.
Better to remove this comment, or update it to account for the size discrepancy due to mmaped storage.
We can also introduce a factor to multiply this module size, to get a better estimation using the old method.
There was a problem hiding this comment.
Okay, good point. Thank you!
I created #1666 for this such that we don't get blocked here.
I'll remove this comment and leave estimate_module_size to be updated.
There was a problem hiding this comment.
Oh, also store is suppsed to do that: Returns the size of the serialized module.. Which is a good point. The file system cache should not worry about the in-memory size estimation.
|
I see |
The remaining usages are needed to keep the current Wasmer 2.3 compiling. We don't use them explicitly. When we upgrade to Wasmer 3, they will go away. |
This reverts #1041 and prepares the Wasmer 3 upgrade because loupe was refectored aways from Wasmer and is not available anymore.