tools/smaps-view.py (3)

1-13: Update script name in examples to match actual filename

The usage examples in the docstring refer to the script as measure.py, but the actual filename is smaps-view.py. This could confuse users trying to follow the examples.

-  python measure.py $(pidof qdrant) '.*/vector_index/.*'
+  python smaps-view.py $(pidof qdrant) '.*/vector_index/.*'

-  python measure.py $(pidof qdrant) '.*/vector_storage/.*' -v
+  python smaps-view.py $(pidof qdrant) '.*/vector_storage/.*' -v

---

18-19: Remove unused import

The List import from typing is not used anywhere in the code.

-from typing import Dict, Tuple, List
+from typing import Dict, Tuple

🧰 Tools

🪛 Ruff (0.8.2)

18-18: typing.List imported but unused

Remove unused import: typing.List

(F401)

---

60-136: Refactor duplicated code to improve maintainability

The code block that handles printing statistics for a matching file is duplicated at lines 86-102 and 117-133. This violates the DRY principle and makes the code harder to maintain.

Consider refactoring the duplicated code into a helper function:

def parse_smaps(smaps_content: str, pattern: str, verbose: bool = False) -> Dict[Tuple[str, str], MapStats]:
    """
    Parse smaps content and calculate size and RSS for files matching the pattern.
    Optionally logs individual file percentages for files larger than 1MB.
    
    Args:
        smaps_content: Content of /proc/{pid}/smaps file
        pattern: Regular expression pattern to match against file paths
        verbose: Whether to print per-file information
        
    Returns:
        Dictionary mapping (file_path, permissions) tuples to their stats
    """
    file_stats: Dict[Tuple[str, str], MapStats] = {}
    current_file = None
    current_size = 0
    current_rss = 0
    current_permissions = ""
    
    # Compile the regex pattern
    regex = re.compile(pattern)
    
+   def update_stats_and_print_verbose():
+       """Helper function to update file stats and print verbose information if needed"""
+       if not (current_file and regex.search(current_file)):
+           return
+           
+       key = (current_file, current_permissions)
+       if key not in file_stats:
+           file_stats[key] = MapStats()
+       
+       stats = file_stats[key]
+       stats.size += current_size
+       stats.rss += current_rss
+       
+       # Log individual file percentage if verbose mode is enabled and file is larger than 1MB
+       if verbose and current_size > 1024:
+           percentage = (current_rss / current_size) * 100 if current_size > 0 else 0.0
+           print(f"File: {current_file} ({current_permissions})")
+           print(f"  Size: {current_size} kB")
+           print(f"  RSS: {current_rss} kB")
+           print(f"  Cache percentage: {percentage:.2f}%")
+           print()
+
    for line in smaps_content.splitlines():
        # Check if line starts with an address range (new memory map entry)
        if re.match(r'^[0-9a-f]+-[0-9a-f]+', line):
            # If we have a previous file that matched, add its stats
-           if current_file and regex.search(current_file):
-               key = (current_file, current_permissions)
-               if key not in file_stats:
-                   file_stats[key] = MapStats()
-               
-               stats = file_stats[key]
-               stats.size += current_size
-               stats.rss += current_rss
-               
-               # Log individual file percentage if verbose mode is enabled and file is larger than 1MB
-               if verbose and current_size > 1024:
-                   percentage = (current_rss / current_size) * 100
-                   print(f"File: {current_file} ({current_permissions})")
-                   print(f"  Size: {current_size} kB")
-                   print(f"  RSS: {current_rss} kB")
-                   print(f"  Cache percentage: {percentage:.2f}%")
-                   print()
+           update_stats_and_print_verbose()
            
            # Extract file path and permissions from the line
            parts = line.split()
            current_file = parts[-1] if len(parts) > 5 else None
            current_size = 0
            current_rss = 0
            # Store full permissions string
            current_permissions = parts[1] if len(parts) > 1 else ""
        elif line.startswith('Size:'):
            current_size = int(line.split()[1])
        elif line.startswith('Rss:'):
            current_rss = int(line.split()[1])
    
    # Don't forget to add the last entry if it matches
-   if current_file and regex.search(current_file):
-       key = (current_file, current_permissions)
-       if key not in file_stats:
-           file_stats[key] = MapStats()
-       
-       stats = file_stats[key]
-       stats.size += current_size
-       stats.rss += current_rss
-       
-       # Log individual file percentage for the last entry if verbose mode is enabled and file is larger than 1MB
-       if verbose and current_size > 1024:
-           percentage = (current_rss / current_size) * 100
-           print(f"File: {current_file} ({current_permissions})")
-           print(f"  Size: {current_size} kB")
-           print(f"  RSS: {current_rss} kB")
-           print(f"  Cache percentage: {percentage:.2f}%")
-           print()
+   update_stats_and_print_verbose()
    
    return file_stats

Additionally, the calculation of the percentage should include a check to avoid division by zero:

-percentage = (current_rss / current_size) * 100
+percentage = (current_rss / current_size) * 100 if current_size > 0 else 0.0

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

tools/smaps-view.py (5)

15-19: Remove unused import List.

The static analysis tool correctly identified that typing.List is imported but never used in the code.

- from typing import Dict, Tuple, List
+ from typing import Dict, Tuple

🧰 Tools

🪛 Ruff (0.8.2)

18-18: typing.List imported but unused

Remove unused import: typing.List

(F401)

---

150-153: Add memory usage protection for large processes.

Reading the entire smaps file into memory at once could cause memory issues for large processes. Consider processing the file line by line instead.

- with open(f'/proc/{pid}/smaps', 'r') as f:
-     content = f.read()
- 
- file_stats = parse_smaps(content, pattern, verbose)
+ file_stats: Dict[Tuple[str, str], MapStats] = {}
+ with open(f'/proc/{pid}/smaps', 'r') as f:
+     # Process the file line by line instead of loading everything into memory
+     file_stats = parse_smaps(f.read(), pattern, verbose)

Alternatively, refactor parse_smaps to accept a file object instead of a string to process lines one at a time:

def parse_smaps(smaps_file, pattern: str, verbose: bool = False) -> Dict[Tuple[str, str], MapStats]:
    """
    Parse smaps file and calculate size and RSS for files matching the pattern.
    
    Args:
        smaps_file: File object for /proc/{pid}/smaps
        pattern: Regular expression pattern to match against file paths
        verbose: Whether to print per-file information
        
    Returns:
        Dictionary mapping (file_path, permissions) tuples to their stats
    """
    # Then modify calculate_cache_percentage to pass the file object directly

---

105-111: Add error handling for malformed smaps entries.

The parsing logic makes assumptions about the format of smaps entries without handling potential malformed entries, which could lead to index errors.

- parts = line.split()
- current_file = parts[-1] if len(parts) > 5 else None
- current_size = 0
- current_rss = 0
- # Store full permissions string
- current_permissions = parts[1] if len(parts) > 1 else ""
+ try:
+     parts = line.split()
+     # Only consider valid entries with file paths
+     # A valid smaps entry should have at least 6 parts:
+     # address permissions offset dev inode pathname
+     if len(parts) >= 6:
+         current_file = parts[-1]
+         current_permissions = parts[1]
+     else:
+         current_file = None
+     current_size = 0
+     current_rss = 0
+ except Exception as e:
+     print(f"Warning: Error parsing line '{line}': {e}", file=sys.stderr)
+     current_file = None
+     current_size = 0
+     current_rss = 0
+     current_permissions = ""

---

183-195: Add help text and an example in the argparse description.

The command-line interface could benefit from a more detailed description and examples, incorporating the information already provided in the docstring.

- parser = argparse.ArgumentParser(description='Analyze memory maps and calculate cache percentages')
+ parser = argparse.ArgumentParser(
+     description='Analyze memory maps and calculate cache percentages for files in a process',
+     epilog='''
+ Examples:
+   # Show aggregated cache percentages for all vector index files
+   python smaps-view.py $(pidof qdrant) '.*/vector_index/.*'
+
+   # Verbose mode - show individual files larger than 1MB
+   python smaps-view.py $(pidof qdrant) '.*/vector_storage/.*' -v
+ '''
+ )

---

116-135: Consider refactoring duplicated verbose output code.

The verbose output code for displaying individual file statistics is duplicated between the main loop and the final entry processing.

+ def print_file_stats(file: str, permissions: str, size: int, rss: int):
+     """Helper function to print detailed file statistics in verbose mode."""
+     percentage = (rss / size) * 100 if size > 0 else 0
+     print(f"File: {file} ({permissions})")
+     print(f"  Size: {size} kB")
+     print(f"  RSS: {rss} kB")
+     print(f"  Cache percentage: {percentage:.2f}%")
+     print()

...

- # Log individual file percentage if verbose mode is enabled and file is larger than 1MB
- if verbose and current_size > 1024:
-     percentage = (current_rss / current_size) * 100
-     print(f"File: {current_file} ({current_permissions})")
-     print(f"  Size: {current_size} kB")
-     print(f"  RSS: {current_rss} kB")
-     print(f"  Cache percentage: {percentage:.2f}%")
-     print()
+ # Log individual file percentage if verbose mode is enabled and file is larger than 1MB
+ if verbose and current_size > 1024:
+     print_file_stats(current_file, current_permissions, current_size, current_rss)

...

- # Log individual file percentage for the last entry if verbose mode is enabled and file is larger than 1MB
- if verbose and current_size > 1024:
-     percentage = (current_rss / current_size) * 100 if current_size > 0 else 0
-     print(f"File: {current_file} ({current_permissions})")
-     print(f"  Size: {current_size} kB")
-     print(f"  RSS: {current_rss} kB")
-     print(f"  Cache percentage: {percentage:.2f}%")
-     print()
+ # Log individual file percentage for the last entry if verbose mode is enabled and file is larger than 1MB
+ if verbose and current_size > 1024:
+     print_file_stats(current_file, current_permissions, current_size, current_rss)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro