Skip to content

Commit e4e6dc4

Browse files
skruegermeta-codesync[bot]
authored andcommitted
build: Move StringCore include and computeAndSetIsAscii to cpp (facebookincubator#16638)
Summary: Pull Request resolved: facebookincubator#16638 SimpleVector.h is included by many translation units. Moving the computeAndSetIsAscii method body to the .cpp file via explicit template specialization reduces compile-time work for every includer. The SFINAE guard already ensures the method only compiles for T = StringView, so an explicit specialization for SimpleVector<StringView>::computeAndSetIsAscii<StringView> is the natural approach. AUTODEPS moved the velox_functions_string dependency from exported_deps to deps since the header no longer needs it. Differential Revision: D94973899 fbshipit-source-id: f9e1641dd13b78bfc62a74ba9b6ebbacd9deac93
1 parent a6f53d4 commit e4e6dc4

File tree

4 files changed

+38
-28
lines changed

4 files changed

+38
-28
lines changed

velox/expression/CastExpr-inl.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "velox/common/base/Exceptions.h"
2020
#include "velox/core/CoreTypeSystem.h"
2121
#include "velox/expression/StringWriter.h"
22+
#include "velox/functions/lib/string/StringCore.h"
2223
#include "velox/type/Type.h"
2324
#include "velox/vector/SelectivityVector.h"
2425

velox/python/legacy/pyvelox.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@
1919
#include "conversion.h"
2020
#include "serde.h"
2121
#include "signatures.h"
22+
#include "velox/external/utf8proc/utf8procImpl.h"
23+
24+
#ifndef PYVELOX_VERSION
25+
#define PYVELOX_VERSION dev
26+
#endif
2227

2328
namespace facebook::velox::py {
2429
using namespace velox;

velox/vector/SimpleVector.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
#include "velox/vector/SimpleVector.h"
18+
#include "velox/functions/lib/string/StringCore.h"
1819

1920
namespace facebook::velox {
2021

@@ -30,4 +31,34 @@ void SimpleVector<StringView>::validate(
3031
}
3132
}
3233

34+
template <>
35+
template <>
36+
bool SimpleVector<StringView>::computeAndSetIsAscii<StringView>(
37+
const SelectivityVector& rows) {
38+
if (rows.isSubset(*asciiInfo.readLockedAsciiComputedRows())) {
39+
return asciiInfo.isAllAscii();
40+
}
41+
ensureIsAsciiCapacity();
42+
bool isAllAscii = true;
43+
rows.applyToSelected([&](auto row) {
44+
if (!isNullAt(row)) {
45+
auto string = valueAt(row);
46+
isAllAscii &=
47+
functions::stringCore::isAscii(string.data(), string.size());
48+
}
49+
});
50+
51+
auto wlockedAsciiComputedRows = asciiInfo.writeLockedAsciiComputedRows();
52+
if (!wlockedAsciiComputedRows->hasSelections()) {
53+
asciiInfo.setIsAllAscii(isAllAscii);
54+
} else {
55+
asciiInfo.setIsAllAscii(asciiInfo.isAllAscii() & isAllAscii);
56+
}
57+
58+
wlockedAsciiComputedRows->select(rows);
59+
asciiInfo.setAsciiComputedRowsEmpty(
60+
!wlockedAsciiComputedRows->hasSelections());
61+
return asciiInfo.isAllAscii();
62+
}
63+
3364
} // namespace facebook::velox

velox/vector/SimpleVector.h

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
#include <folly/hash/Hash.h>
2828
#include <glog/logging.h>
2929

30-
#include "velox/functions/lib/string/StringCore.h"
3130
#include "velox/type/DecimalUtil.h"
3231
#include "velox/type/FloatingPointUtil.h"
3332
#include "velox/type/Type.h"
@@ -310,33 +309,7 @@ class SimpleVector : public BaseVector {
310309
/// present. Returns computed value.
311310
template <typename U = T>
312311
typename std::enable_if_t<std::is_same_v<U, StringView>, bool>
313-
computeAndSetIsAscii(const SelectivityVector& rows) {
314-
if (rows.isSubset(*asciiInfo.readLockedAsciiComputedRows())) {
315-
return asciiInfo.isAllAscii();
316-
}
317-
ensureIsAsciiCapacity();
318-
bool isAllAscii = true;
319-
rows.applyToSelected([&](auto row) {
320-
if (!isNullAt(row)) {
321-
auto string = valueAt(row);
322-
isAllAscii &=
323-
functions::stringCore::isAscii(string.data(), string.size());
324-
}
325-
});
326-
327-
// Set isAllAscii flag, it will unset if we encounter any utf.
328-
auto wlockedAsciiComputedRows = asciiInfo.writeLockedAsciiComputedRows();
329-
if (!wlockedAsciiComputedRows->hasSelections()) {
330-
asciiInfo.setIsAllAscii(isAllAscii);
331-
} else {
332-
asciiInfo.setIsAllAscii(asciiInfo.isAllAscii() & isAllAscii);
333-
}
334-
335-
wlockedAsciiComputedRows->select(rows);
336-
asciiInfo.setAsciiComputedRowsEmpty(
337-
!wlockedAsciiComputedRows->hasSelections());
338-
return asciiInfo.isAllAscii();
339-
}
312+
computeAndSetIsAscii(const SelectivityVector& rows);
340313

341314
/// Clears asciiness state.
342315
template <typename U = T>

0 commit comments

Comments
 (0)