[04:17:14] +Dumcord: <b​aktus_79> Hey, quick question. Which is more efficient? DataStore#getClaimAt or DataStore#getClaim (edited)
[04:18:34] +Dumcord: <b​aktus_79> It just seems like DataStore#getClaim iterates through an arraylist and DataStore#getClaimAt has a concurrentHashMap for its storage?
[06:25:28] +Dumcord: <e​vident._.> #2361 (comment) kind of weird that was just talked about in an issue
[09:13:17] +Dumcord: <b​aktus_79> haha, I am the author of BanFromClaim. I cant see how DataStore#getClaimAt should be logically more efficient.
[09:15:45] +Dumcord: <b​aktus_79> They should go through the same list I guess? Doesnt DataStore#getClaimAt have a few more steps to get the claim?
[09:27:47] +Dumcord: <r​obomwm> Hashmap is faster than iterating an array
[10:23:24] @RoboMWM: getclaimat gets only a subset of possible claims (via hashmap call, O(1)) in the chunk of the provided location, rather than iterating the entire list of claims (via iteration, O(n))
[10:24:03] @RoboMWM: it may seem like more steps, but it's far faster and can be seen as such via big O notation
[10:25:19] @RoboMWM: if we sorted the arraylist, we could also make it an O(1) lookup (would have to account for deleted claims tho), but that would be moot since we do plan to change how claims are identified
[10:25:39] @RoboMWM: we could also map claims to id too, but I don't often see the use case for that.
[10:25:53] @RoboMWM: or rather see any use case for that
[13:37:54] +Dumcord: <b​aktus_79> Thanks for the answer, RoboMWM. I didnt know they operated different when obtaining the claim. I always thought that by getting it by providing the claim id would be way faster. But again, thanks for the insight 🙂

Originally posted in #2361 (comment)

Basically should we just deprecate it with javadoc warning or is it worth it to cache this data? I don't see many use cases for the latter. I'd presume most plugin developers should be getting via location anyways, and those that aren't are incorrectly assuming getClaimAt is more expensive.

If it's not needed/no reasonable use cases, then any plugin that really needs this functionality can just store their own cache with getClaims. Otherwise could move this to be getClaimId.

Either way, would at the very least refactor getClaim to use getClaimAt logic since that's what we primarily want addons to call anyway.

Honestly, it's not a huge difference for GP to cache - it's the difference between the current ArrayList and a Map (or better, a Long2ObjMap from FastUtil) for storing active claims. I rolled this into #2295, though that has some problems related to deleting groups of claims that I need to go back and address when life is less hectic. The largest nuisance is that the map would need to include subclaims to prevent a regression on being able to look up subclaims by ID. In that regard, it may be easier to hold off on this until subclaims are deleted.
Alternately, we could maintain a separate map and update it whenever relevant, though then we waste memory on duplicate entries.

In that regard, it may be easier to hold off on this until subclaims are deleted.

Yes, that is definitely the plan, in v18.