Store a hash in a NormalizedUri #214
Conversation
NormalizedUris are supposed to be used as keys in maps. Using a HashMap with a cached key is far faster than using a Map with the slow ordering instance. In particular, ghcide was hammering updateDiagnostics and the `compare` function for `NormalizedUri` was dominating the profile.
| where (Uri t) = maybe uri filePathToUri (uriToFilePath uri) | ||
| -- To ensure all `Uri`s have the file path like the created ones by `filePathToUri` | ||
| norm = T.pack $ escapeURIString isUnescapedInURI $ unEscapeString $ T.unpack t |
There was a problem hiding this comment.
given this is just a value for indexing, maybe we should skip the T.pack, and store a string?
There was a problem hiding this comment.
Hmm, that would increase the memory footprint quite a bit. I wold prefer to implement the normalisation logic directly on text.
| -- | ||
| -- If you care about performance then you should use a hash map. The keys | ||
| -- are cached in order to make hashing very fast. | ||
| data NormalizedUri = NormalizedUri Int !Text |
There was a problem hiding this comment.
Is the hash intentionally non-strict? I guess it saves you a bit if you don’t care about it but if you do care about it, making it strict so it can be unboxed seems benefitiial.
There was a problem hiding this comment.
You want to delay the hashing until it is actually used, which may never happen. So it makes sense to keep it non-strict
There was a problem hiding this comment.
The hash is intentionally non-strict for the reasons that Pepe suggests.
|
Text has an efficient |
|
The hashing method still accounts for 2.8% of runtime if you don't cache it so I would prefer to leave it as it is for now. |
NormalizedUris are supposed to be used as keys in maps. Using a HashMap
with a cached key is far faster than using a Map with the slow ordering
instance.
In particular, ghcide was hammering updateDiagnostics and the
comparefunction for
NormalizedUriwas dominating the profile.