Bug 218062 - Replace O(n^2) algorithm from r268709 with O(n) algorithm
Summary: Replace O(n^2) algorithm from r268709 with O(n) algorithm
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-10-21 16:49 PDT by Alex Christensen
Modified: 2021-03-16 11:01 PDT (History)
3 users (show)

See Also:


Attachments
Patch (2.90 KB, patch)
2020-10-21 16:53 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2020-10-21 16:49:24 PDT
Replace O(n^2) algorithm from r268709 with O(n) algorithm
Comment 1 Alex Christensen 2020-10-21 16:53:02 PDT
Created attachment 412047 [details]
Patch
Comment 2 EWS 2020-10-21 21:50:38 PDT
Committed r268852: <https://trac.webkit.org/changeset/268852>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 412047 [details].
Comment 3 Radar WebKit Bug Importer 2020-10-21 21:51:26 PDT
<rdar://problem/70559804>
Comment 4 Geoffrey Garen 2020-10-22 10:45:52 PDT
Comment on attachment 412047 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=412047&action=review

> Source/WebCore/bindings/js/JSDOMConvertRecord.h:141
> +                        auto iterator = resultMap.find(typedKey);
> +                        if (iterator != resultMap.end()) {
> +                            ASSERT(result[iterator->value].key == typedKey);
> +                            result[iterator->value].value = WTFMove(typedValue);
>                              continue;
>                          }
> +                        resultMap.add(typedKey, result.size());

You can avoid the double hash lookup here by doing the add first and then checking its result:

auto addResult = resultMap.add(typedKey, result.size());
if (!addResult.isNewEntry) {
    ASSERT(result[addResult.iterator->value].key == typedKey);
    result[addResult.iterator->value].value = WTFMove(typedValue);
    continue;
}

(The default behavior of add() does not overwrite existing entries.)

Also: Why are you storing both the key and the value in both data structures? Seems like putting just the key into a HashSet would be slightly more efficient.
Comment 5 Alexey Shvayka 2021-03-16 11:01:00 PDT
(In reply to Geoffrey Garen from comment #4)
> You can avoid the double hash lookup here by doing the add first and then
> checking its result:

I've applied this comment as part of https://bugs.webkit.org/show_bug.cgi?id=223219. Thanks!

> Also: Why are you storing both the key and the value in both data
> structures? Seems like putting just the key into a HashSet would be slightly
> more efficient.

We store an index of the first |key| occurrence as the |value| so we can update it when the duplicate is found.