Title: [290846] trunk
- Revision
- 290846
- Author
- cdu...@apple.com
- Date
- 2022-03-04 14:10:10 -0800 (Fri, 04 Mar 2022)
Log Message
URL's isolatedCopy() optimization when called on a r-value reference doesn't work
https://bugs.webkit.org/show_bug.cgi?id=237481
Reviewed by Geoffrey Garen.
Source/WTF:
URL has an isolatedCopy() implementation that attempts to optimize the case where
it is called on a r-value reference. The idea is to rely on the String's
isolatedCopy() implementation which is optimized when called on a r-value reference.
Note that there are some specific conditions under which the String implementation
is able to avoid the copy (see String::isSafeToSendToAnotherThread()).
Namely, the StringImpl's refcount needs to be 1 and it cannot be backed by an
AtomStringImpl.
The issue was that URL::isolatedCopy() would first copy the URL, which would copy
its m_string and thus bump its refcount. As a result, m_string's refcount could
never be 1 and the optimization could never kick in.
* wtf/URL.cpp:
(WTF::URL::isolatedCopy):
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WTF/URL.cpp:
(TestWebKitAPI::TEST_F):
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (290845 => 290846)
--- trunk/Source/WTF/ChangeLog 2022-03-04 22:08:20 UTC (rev 290845)
+++ trunk/Source/WTF/ChangeLog 2022-03-04 22:10:10 UTC (rev 290846)
@@ -1,3 +1,26 @@
+2022-03-04 Chris Dumez <cdu...@apple.com>
+
+ URL's isolatedCopy() optimization when called on a r-value reference doesn't work
+ https://bugs.webkit.org/show_bug.cgi?id=237481
+
+ Reviewed by Geoffrey Garen.
+
+ URL has an isolatedCopy() implementation that attempts to optimize the case where
+ it is called on a r-value reference. The idea is to rely on the String's
+ isolatedCopy() implementation which is optimized when called on a r-value reference.
+
+ Note that there are some specific conditions under which the String implementation
+ is able to avoid the copy (see String::isSafeToSendToAnotherThread()).
+ Namely, the StringImpl's refcount needs to be 1 and it cannot be backed by an
+ AtomStringImpl.
+
+ The issue was that URL::isolatedCopy() would first copy the URL, which would copy
+ its m_string and thus bump its refcount. As a result, m_string's refcount could
+ never be 1 and the optimization could never kick in.
+
+ * wtf/URL.cpp:
+ (WTF::URL::isolatedCopy):
+
2022-03-02 Alex Christensen <achristen...@webkit.org>
Add SPI _WKDataTask
Modified: trunk/Source/WTF/wtf/URL.cpp (290845 => 290846)
--- trunk/Source/WTF/wtf/URL.cpp 2022-03-04 22:08:20 UTC (rev 290845)
+++ trunk/Source/WTF/wtf/URL.cpp 2022-03-04 22:10:10 UTC (rev 290846)
@@ -87,7 +87,7 @@
URL URL::isolatedCopy() &&
{
- URL result = *this;
+ URL result { WTFMove(*this) };
result.m_string = WTFMove(result.m_string).isolatedCopy();
return result;
}
Modified: trunk/Tools/ChangeLog (290845 => 290846)
--- trunk/Tools/ChangeLog 2022-03-04 22:08:20 UTC (rev 290845)
+++ trunk/Tools/ChangeLog 2022-03-04 22:10:10 UTC (rev 290846)
@@ -1,3 +1,15 @@
+2022-03-04 Chris Dumez <cdu...@apple.com>
+
+ URL's isolatedCopy() optimization when called on a r-value reference doesn't work
+ https://bugs.webkit.org/show_bug.cgi?id=237481
+
+ Reviewed by Geoffrey Garen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WTF/URL.cpp:
+ (TestWebKitAPI::TEST_F):
+
2022-03-04 Aditya Keerthi <akeer...@apple.com>
[iOS] Unable to scroll to a found text range when there is an existing selection
Modified: trunk/Tools/TestWebKitAPI/Tests/WTF/URL.cpp (290845 => 290846)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/URL.cpp 2022-03-04 22:08:20 UTC (rev 290845)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/URL.cpp 2022-03-04 22:10:10 UTC (rev 290846)
@@ -567,4 +567,20 @@
EXPECT_EQ(url13.string(), url8.string());
}
+TEST_F(WTF_URL, IsolatedCopy)
+{
+ URL url1 { "http://www.apple.com"_str };
+ auto* originalStringImpl = url1.string().impl();
+ URL url1Copy = url1.isolatedCopy();
+ EXPECT_EQ(url1Copy, url1);
+ EXPECT_NE(url1Copy.string().impl(), originalStringImpl); // Should have done a deep copy of the String.
+
+ // Tests optimization for URL::isolatedCopy() on a r-value reference.
+ URL url2 { "https://www.webkit.org"_str };
+ originalStringImpl = url2.string().impl();
+ URL url2Copy = WTFMove(url2).isolatedCopy();
+ EXPECT_EQ(url2Copy, URL { "https://www.webkit.org"_str });
+ EXPECT_EQ(url2Copy.string().impl(), originalStringImpl); // Should have adopted the StringImpl of url2.
+}
+
} // namespace TestWebKitAPI
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
