Bug 192814 - clang-tidy: Use const reference for MediaTime parameter to prevent object copy
Summary: clang-tidy: Use const reference for MediaTime parameter to prevent object copy
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-12-18 11:03 PST by David Kilzer (:ddkilzer)
Modified: 2018-12-18 13:54 PST (History)
12 users (show)

See Also:


Attachments
Patch v1 (2.45 KB, patch)
2018-12-18 11:04 PST, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-sierra-wk2 (3.31 MB, application/zip)
2018-12-18 12:49 PST, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2018-12-18 11:03:00 PST
Running `clang-tidy -header-filter=.* -checks='-*,performance-*,-performance-noexcept-*' ...` on JavaScriptCore and WebCore source files found these unnecessary object copies in MediaTime function parameters in trunk revision r239112.

Source/JavaScriptCore/runtime/JSCJSValue.h:554:42: warning: the parameter 't' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
ALWAYS_INLINE JSValue jsNumber(MediaTime t)
                                         ^
                               const    &

Source/WebCore/bindings/js/JSDOMConvertNumbers.h:392:43: warning: the parameter 'value' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
    static JSC::JSValue convert(MediaTime value)
                                          ^
                                const    &
Comment 1 David Kilzer (:ddkilzer) 2018-12-18 11:04:26 PST
Created attachment 357582 [details]
Patch v1
Comment 2 Mark Lam 2018-12-18 11:19:24 PST
Comment on attachment 357582 [details]
Patch v1

LGTM if EWS bots are happy.
Comment 3 EWS Watchlist 2018-12-18 12:49:23 PST
Comment on attachment 357582 [details]
Patch v1

Attachment 357582 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10461730

New failing tests:
fast/forms/datalist/datalist-textinput-suggestions-order.html
Comment 4 EWS Watchlist 2018-12-18 12:49:25 PST
Created attachment 357599 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 5 David Kilzer (:ddkilzer) 2018-12-18 13:27:09 PST
(In reply to Build Bot from comment #4)
> Created attachment 357599 [details]
> Archive of layout-test-results from ews104 for mac-sierra-wk2
> 
> The attached test failures were seen while running run-webkit-tests on the
> mac-wk2-ews.
> Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

On further analysis, this appears to be a rare flaky crash that's unrelated to this change.  I filed <rdar://problem/46819769> to track the crash internally.
Comment 6 Radar WebKit Bug Importer 2018-12-18 13:28:55 PST
<rdar://problem/46820023>
Comment 7 WebKit Commit Bot 2018-12-18 13:54:32 PST
Comment on attachment 357582 [details]
Patch v1

Clearing flags on attachment: 357582

Committed r239352: <https://trac.webkit.org/changeset/239352>
Comment 8 WebKit Commit Bot 2018-12-18 13:54:34 PST
All reviewed patches have been landed.  Closing bug.