Introduce makeAtomString() which is an optimized version of makeString() when the caller knows it wants an AtomString. It may avoid a StringImpl allocation when the string is already in the AtomStringTable.
Created attachment 457820 [details] Patch
Created attachment 457824 [details] Patch
Comment on attachment 457824 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=457824&action=review > Source/WTF/wtf/text/StringConcatenate.h:502 > + if (areAllAdapters8Bit) { > + LChar buffer[maxLengthToUseStackVariable]; > + stringTypeAdapterAccumulator(buffer, adapter, adapters...); > + return AtomString { buffer, length }; > + } > + UChar buffer[maxLengthToUseStackVariable]; > + stringTypeAdapterAccumulator(buffer, adapter, adapters...); > + return AtomString { buffer, length }; I wonder if the compiler is smart enough to not allocate both buffers. > Source/WTF/wtf/text/StringConcatenate.h:525 > + if (areAllAdapters8Bit) { > + LChar* buffer; > + RefPtr<StringImpl> resultImpl = StringImpl::tryCreateUninitialized(length, buffer); > + if (!resultImpl) > + return nullAtom(); > + > + if (buffer) > + stringTypeAdapterAccumulator(buffer, adapter, adapters...); > + > + return resultImpl.get(); > + } > + > + UChar* buffer; > + RefPtr<StringImpl> resultImpl = StringImpl::tryCreateUninitialized(length, buffer); > + if (!resultImpl) > + return nullAtom(); > + > + if (buffer) > + stringTypeAdapterAccumulator(buffer, adapter, adapters...); > + > + return resultImpl.get(); Is there a way to share code with tryMakeStringFromAdapters here?
Created attachment 457836 [details] Patch
Created attachment 457837 [details] Patch
Created attachment 457838 [details] Patch
Comment on attachment 457838 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=457838&action=review > Source/WebCore/svg/SVGLengthValue.cpp:30 > +#include <wtf/text/StringConcatenate.h> > #include <wtf/text/StringConcatenateNumbers.h> I thought "StringConcatenateNumbers.h" included "StringConcatenate.h"
(In reply to Darin Adler from comment #7) > Comment on attachment 457838 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=457838&action=review > > > Source/WebCore/svg/SVGLengthValue.cpp:30 > > +#include <wtf/text/StringConcatenate.h> > > #include <wtf/text/StringConcatenateNumbers.h> > > I thought "StringConcatenateNumbers.h" included "StringConcatenate.h" Oh, I think that was me trying to fix a build error the wrong way. I'll drop.
Created attachment 457841 [details] Patch
(In reply to Darin Adler from comment #3) > Comment on attachment 457824 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=457824&action=review > > > Source/WTF/wtf/text/StringConcatenate.h:502 > > + if (areAllAdapters8Bit) { > > + LChar buffer[maxLengthToUseStackVariable]; > > + stringTypeAdapterAccumulator(buffer, adapter, adapters...); > > + return AtomString { buffer, length }; > > + } > > + UChar buffer[maxLengthToUseStackVariable]; > > + stringTypeAdapterAccumulator(buffer, adapter, adapters...); > > + return AtomString { buffer, length }; > > I wonder if the compiler is smart enough to not allocate both buffers. Good question. I am not quite sure how to find out though.
Comment on attachment 457841 [details] Patch Very nice.
Committed r293024 (249763@main): <https://commits.webkit.org/249763@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 457841 [details].
<rdar://problem/91972473>