Skip to content

Conversation

@cesco69
Copy link
Contributor

@cesco69 cesco69 commented Jan 14, 2026

just a micro-optimization.

Reduces overhead in hot LRU operations by avoiding has() checks when a direct cache-entry lookup is sufficient.

@augmentcode
Copy link

augmentcode bot commented Jan 14, 2026

🤖 Augment PR Summary

Summary: Reduces overhead in hot LRU operations by avoiding has() checks when a direct entry lookup is sufficient.

Changes: delete() and expiresAt() now gate on this.items[key] presence and reuse the retrieved item.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 1 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

@cesco69
Copy link
Contributor Author

cesco69 commented Jan 14, 2026

augment review

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@avoidwork
Copy link
Owner

avoidwork commented Jan 14, 2026

that's previously been a deopt due to the type assignment and verification. do you have an IR render that shows it's ok? i used to use irhydra, and then deoptigate to verify that path.

@cesco69
Copy link
Contributor Author

cesco69 commented Jan 15, 2026

that's previously been a deopt due to the type assignment and verification.

This is possible because the variable item become megamorphic: If the property read site sees more than four different class shapes, it is known as Megamorphic and is always handled by the slow path.

do you have an IR render that shows it's ok? i used to use irhydra, and then deoptigate to verify that path.

no, sorry

@avoidwork
Copy link
Owner

avoidwork commented Jan 15, 2026

the item isn't megamorphic when the deopt occurs, it's actually early in the life cycle and the has() is a strategy for a long running instance.

you're basically swapping the in operator for an identity check. there's no obvious gain from it from a design pov, but engines evolve so i'm curious if it's still an issue.

to clarify; i already did that work, so if you want to land the change you would need to do the new work to show it's safe.

@avoidwork
Copy link
Owner

i should also clarify, that the items object does go megamorphic eventually (depending on usage), but the current code is still faster than a Map or other assumed optimizations based on how those structs are implemented.

you need a good reason to propose a subtle, but significant change, and benchmarks.

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