Skip to content

Linux pagecache.Files: Memory Usage#1769

Merged
ikelos merged 5 commits intodevelopfrom
inodepages/memory_usage
Apr 15, 2025
Merged

Linux pagecache.Files: Memory Usage#1769
ikelos merged 5 commits intodevelopfrom
inodepages/memory_usage

Conversation

@dgmcdona
Copy link
Copy Markdown
Contributor

Converts objects.Pointer to int before storing them in the set. This
should have a substantial impact on memory, similar to those in #1758

@dgmcdona
Copy link
Copy Markdown
Contributor Author

On second thought, the calls to int() for seen_dentries may not be necessary since dentry.vol.offset is just an int already, but maybe good to leave it explicit?

@dgmcdona dgmcdona marked this pull request as draft April 10, 2025 21:04
@dgmcdona dgmcdona force-pushed the inodepages/memory_usage branch from 28837e3 to 0bf6c75 Compare April 10, 2025 21:27
@dgmcdona dgmcdona marked this pull request as ready for review April 10, 2025 21:35
@dgmcdona dgmcdona force-pushed the inodepages/memory_usage branch from 0bf6c75 to 1c11cd2 Compare April 10, 2025 21:36
Copy link
Copy Markdown
Member

@ikelos ikelos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, although, please could we add a couple comments to explain why we're casting in this specific instance when we normally go to great lengths to get people not to cast (so that they don't lose useful information or litter the code with unnecessary casts)...

Copy link
Copy Markdown
Member

@ikelos ikelos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of interest, does the if root_inode_ptr in seen_inodes: still work, if the entries in seen_inodes are all ints? Just curious to know whether it does object comparison or runs the equality tests on them (and whether our equality test says they're the same). Either way I'm happy with this, but I'll hold off until we get the answer, just in case we can trim one of the casts...

Converts `objects.Pointer` to `int` before storing them in the set. This
should have a substantial impact on memory, similar to those in #1758
`dentry.vol.offset` is already a basic Python `int`.
Adds a couple of comments explaining why we're doing a lossy conversion
to Python `int` (saving memory).
Removes casts to `int` performed before checking set membership, since
the computed `__hash__` value will be the same for both the Python
primitive and the volatility `objects.Pointer`.
@dgmcdona
Copy link
Copy Markdown
Contributor Author

dgmcdona commented Apr 14, 2025

Looks like the call to __hash__ returns the same value for both, and the equality test passes:

[ins] In [1]: seen = set()

[ins] In [2]: seen.add(int(root_inode_ptr))

[ins] In [3]: seen
Out[3]: {175402173655680}

[ins] In [4]: root_inode_ptr in seen
Out[4]: True

[ins] In [5]: root_inode_ptr.__class__
Out[5]: volatility3.framework.objects.Pointer

[ins] In [6]: hash(root_inode_ptr)
Out[6]: 175402173655680

[ins] In [7]: hash(int(root_inode_ptr))
Out[7]: 175402173655680

[ins] In [8]: int(root_inode_ptr) == root_inode_ptr
Out[8]: True

@dgmcdona dgmcdona force-pushed the inodepages/memory_usage branch from bd5f843 to a236c0d Compare April 14, 2025 23:33
@dgmcdona
Copy link
Copy Markdown
Contributor Author

Removed the unneeded casts in a236c0d

@ikelos ikelos merged commit 8dae65e into develop Apr 15, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants