Skip to content

Conversation

@qiweiii
Copy link
Contributor

@qiweiii qiweiii commented Sep 4, 2025

~2x speed improve

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

The memory implementation has been significantly refactored for performance, primarily by replacing dictionary-based page lookups with a bitmask implementation and using unsafe memory operations for faster data handling. While the changes are generally positive for performance, a potential memory safety issue has been introduced in StandardMemory where a read can go out of bounds. Additionally, a piece of logic in GeneralMemory for updating memory chunks was made more complex and less efficient than its previous implementation.

@codecov
Copy link

codecov bot commented Sep 4, 2025

Codecov Report

❌ Patch coverage is 87.50000% with 120 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.66%. Comparing base (3bfa3b6) to head (ae12c54).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
PolkaVM/Sources/PolkaVM/Memory/PageMap.swift 71.77% 81 Missing ⚠️
PolkaVM/Sources/PolkaVM/Memory/Memory.swift 82.25% 11 Missing ⚠️
...aVM/Sources/PolkaVM/Executors/JIT/VMStateJIT.swift 0.00% 10 Missing ⚠️
PolkaVM/Sources/PolkaVM/Memory/GeneralMemory.swift 97.37% 7 Missing ⚠️
PolkaVM/Sources/PolkaVM/VMStateInterpreter.swift 77.77% 4 Missing ⚠️
...olkaVM/Sources/PolkaVM/Memory/StandardMemory.swift 98.32% 3 Missing ⚠️
PolkaVM/Sources/PolkaVM/Registers.swift 94.23% 3 Missing ⚠️
Codec/Sources/Codec/IntegerCodec.swift 96.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #364      +/-   ##
==========================================
- Coverage   81.68%   81.66%   -0.02%     
==========================================
  Files         378      381       +3     
  Lines       33362    33572     +210     
==========================================
+ Hits        27251    27416     +165     
- Misses       6111     6156      +45     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@qiweiii qiweiii marked this pull request as ready for review September 4, 2025 15:17
@github-actions
Copy link

github-actions bot commented Sep 4, 2025

No issues found

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

The changes introduce significant performance improvements across memory management, instruction caching, and register access by using more direct memory manipulation (e.g., memcpy, bitfields, pointer arithmetic) and reducing overhead from data structures. However, there is a potential integer overflow bug in the new PageMap implementation when searching for free memory pages.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

The pull request introduces significant performance improvements, primarily by refactoring the memory management system in PolkaVM. The changes include replacing dictionary-based page tracking with a more efficient bitmask implementation, utilizing memcpy and memset for faster memory operations, and removing the LRUCache dependency. Additional performance gains are achieved by optimizing dictionary capacity reservations and reusing an expensive DateFormatter object. One minor code redundancy was found.

@github-actions
Copy link

github-actions bot commented Sep 5, 2025

No issues found

@qiweiii qiweiii enabled auto-merge (squash) September 5, 2025 03:07
@qiweiii qiweiii merged commit cfe98d5 into master Sep 5, 2025
5 of 6 checks passed
@qiweiii qiweiii deleted the pvm--improve branch September 5, 2025 03:53
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.

3 participants