[PATCH] D126159: [ADT] Add edit_distance_insensitive to StringRef

[Nathan James via Phabricator via llvm-commits]

njames93 marked 4 inline comments as done.
njames93 added inline comments.


================
Comment at: llvm/include/llvm/ADT/edit_distance.h:86-91
+            Previous + (Map(FromArray[y - 1]) == Map(ToArray[x - 1]) ? 0u : 1u),
+            std::min(Row[x - 1], Row[x]) + 1);
       }
       else {
-        if (FromArray[y-1] == ToArray[x-1]) Row[x] = Previous;
+        if (Map(FromArray[y - 1]) == Map(ToArray[x - 1]))
+          Row[x] = Previous;
----------------
dblaikie wrote:
> is there any concern about the number of times the map operation is used? Looks like the algorithm visits elements more than once, so might be worth some caching?
> 
> Like an easy one would probably be to compute `Map(FromArray[y-1])` outside the `for x` loop at least? (but maybe other caching should be done too, I don't know - I guess toLower is cheap enough that it's not worth much more involved caching? - I guess `Map(ToArray[x])` could be computed and cached for the next round's references to `Map(ToArray[x-1])` for instance?)
Caching the outer loop value makes sense, but for the inner loop, probably not so much. If its expensive then it'd be better off to just create new arrays with the mapped values, then call ComputeEditDistance with no map functor.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126159/new/

https://reviews.llvm.org/D126159