这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@abhinvv1
Copy link

@abhinvv1 abhinvv1 commented Feb 28, 2025

What?

  • Currently, we simply call [element fb_takeSnapshot:NO] when checkStaleness was true.
  • But. if an element was stale and the snapshot method threw an exception (which indicates the element is no longer valid), that exception would propagate upward.
  • Issue: The stale element remained in the cache, meaning that it continued to occupy memory and could be repeatedly accessed or retried, leading to a memory spike over time.
  • This PR is to add removeObjectForKey: method to remove an object from the LRU cache using its key.
  • If [element fb_takeSnapshot:NO] throws an exception (implying the element is stale), the catch block is executed.
  • In the catch block:
    • The stale element is explicitly removed from the cache using a synchronized block.
    • The exception is then re-thrown so that the error handling remains consistent for the calling code.

Impact:

  • Performance remains O(1) for removals i.e. minimal overhead

NSString *reason = [NSString stringWithFormat:@"The element identified by \"%@\" is either not present or it has expired from the internal cache. Try to find it again", uuid];
@throw [NSException exceptionWithName:FBStaleElementException reason:reason userInfo:@{}];
}
if (checkStaleness) {

Choose a reason for hiding this comment

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

would it be possible to add an integration test to validate this behaviour?

Copy link
Author

Choose a reason for hiding this comment

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

can we pick this up later? is it a release blocker? to how much extent do we cover this in tests currently?

Choose a reason for hiding this comment

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

yes, it is ok to do it in a different PR

@abhinvv1 abhinvv1 requested a review from KazuCocoa February 28, 2025 11:51
@mykola-mokhnach mykola-mokhnach merged commit 46dc417 into appium:master Feb 28, 2025
41 of 43 checks passed
github-actions bot pushed a commit that referenced this pull request Feb 28, 2025
## [9.0.6](v9.0.5...v9.0.6) (2025-02-28)

### Bug Fixes

* optimize LRU cache ([#985](#985)) ([46dc417](46dc417))
@github-actions
Copy link

🎉 This PR is included in version 9.0.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants