Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve EvalString #113

Merged
merged 1 commit into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ set(CMAKE_COMPILE_WARNING_AS_ERROR ON)

add_executable(
trimja
src/all.natvis
src/trimja.m.cpp
src/allocationprofiler.cpp
src/basicscope.cpp
Expand Down
38 changes: 38 additions & 0 deletions src/all.natvis
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?xml version="1.0" encoding="utf-8"?>
<AutoVisualizer xmlns="http://schemas.microsoft.com/vstudio/debugger/natvis/2010">
<Type Name="trimja::EvalString">
<Expand>
<CustomListItems MaxItemsPerView="100">
<Variable Name="mask" InitialValue="((trimja::EvalString::Offset)1) &lt;&lt; (sizeof(trimja::EvalString::Offset) * 8 - 1)"/>
<Variable Name="rawLength" InitialValue="(trimja::EvalString::Offset)0"/>
<Variable Name="isVariable" InitialValue="false"/>
<Variable Name="length" InitialValue="(trimja::EvalString::Offset)0"/>
<Variable Name="it" InitialValue="m_data.isShortString() ? m_data._Mypair._Myval2._Bx._Buf : m_data._Mypair._Myval2._Bx._Ptr"/>
<Loop>
<Break Condition="*it == '\0'" />
<Exec>rawLength = *(const trimja::EvalString::Offset*)it</Exec>
<Exec>isVariable = (rawLength &amp; mask)</Exec>
<Exec>length = rawLength &amp; ~mask</Exec>
<If Condition="isVariable">
<Item Name="var">it + sizeof(trimja::EvalString::Offset),[length]s8b</Item>
</If>
<If Condition="!isVariable">
<Item Name="text">it + sizeof(trimja::EvalString::Offset),[length]s8</Item>
</If>
<Exec>it += length + sizeof(length)</Exec>
</Loop>
</CustomListItems>
</Expand>
</Type>

<Type Name="trimja::EvalString::const_iterator">
<Intrinsic Name="isEnd" Expression="*m_pos == '\0'"/>
<Intrinsic Name="rawLength" Expression="*(const trimja::EvalString::Offset*)m_pos"/>
<Intrinsic Name="mask" Expression="(trimja::EvalString::Offset)1 &lt;&lt; (sizeof(trimja::EvalString::Offset) * 8 - 1)"/>
<Intrinsic Name="isVariable" Expression="(rawLength() &amp; mask()) != 0"/>
<Intrinsic Name="length" Expression="rawLength() &amp; ~mask()"/>
<DisplayString Condition="isEnd()">{{ end }}</DisplayString>
<DisplayString Condition="!isEnd() &amp;&amp; isVariable()">{{ var = {m_pos + sizeof(trimja::EvalString::Offset),[length()]s8b} }}</DisplayString>
<DisplayString Condition="!isEnd() &amp;&amp; !isVariable()">{{ text = {m_pos + sizeof(trimja::EvalString::Offset),[length()]s8} }}</DisplayString>
</Type>
</AutoVisualizer>
36 changes: 19 additions & 17 deletions src/evalstring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@
// prefixed with the length of the segment, stored as an `Offset` type. The
// leading bit of this is set to 1 if the segment is a variable, otherwise it is
// 0 if the segment is text. Additionally there is an extra member variable
// `m_lastSegmentLength` that has the same value as the last `Offset`. This
// allows us to jump back to the last segment to extend it if it is a text
// segment.
// `m_lastTextSegmentLength` that has the length of the last text section or 0
// if the last section was not text. This allows us to jump back to the last
// segment to extend it.
//
// This has the benefit that `EvalString` if very cache-friendly when iterating
// and requires only one allocation. Moves and copies should be as cheap as
Expand Down Expand Up @@ -101,18 +101,18 @@ bool operator!=(EvalString::const_iterator lhs,
return !(lhs == rhs);
}

EvalString::EvalString() : m_data(sizeof(Offset), '\0'), m_lastSegmentLength{} {
EvalString::EvalString() : m_data{}, m_lastTextSegmentLength{0} {
assert(empty());
}

void EvalString::clear() {
m_data.assign(sizeof(Offset), '\0');
m_lastSegmentLength = 0;
m_data.clear();
m_lastTextSegmentLength = 0;
assert(empty());
}

bool EvalString::empty() const {
return m_data.size() == sizeof(Offset);
return m_data.empty();
}

EvalString::const_iterator EvalString::begin() const {
Expand All @@ -124,27 +124,29 @@ EvalString::const_iterator EvalString::end() const {
}

void EvalString::appendText(std::string_view text) {
if (!hasLeadingBit(m_lastSegmentLength)) {
assert(!text.empty());
if (m_lastTextSegmentLength > 0) {
// If the last part was plain text we can extend it
const Offset newLength = m_lastSegmentLength + text.size();
const Offset newLength = m_lastTextSegmentLength + text.size();
std::copy_n(reinterpret_cast<const char*>(&newLength), sizeof(newLength),
m_data.end() - sizeof(Offset) - m_lastSegmentLength);
m_data.end() - sizeof(Offset) - m_lastTextSegmentLength);
m_data.append(text);
m_lastSegmentLength = newLength;
m_lastTextSegmentLength = newLength;
} else {
// Otherwise write new segment
m_lastSegmentLength = text.size();
m_data.append(reinterpret_cast<const char*>(&m_lastSegmentLength),
sizeof(m_lastSegmentLength));
const Offset length = text.size();
m_data.append(reinterpret_cast<const char*>(&length), sizeof(length));
m_data.append(text);
m_lastTextSegmentLength = length;
}
}

void EvalString::appendVariable(std::string_view name) {
m_lastSegmentLength = setLeadingBit(name.size());
m_data.append(reinterpret_cast<const char*>(&m_lastSegmentLength),
sizeof(m_lastSegmentLength));
assert(!name.empty());
const Offset length = setLeadingBit(name.size());
m_data.append(reinterpret_cast<const char*>(&length), sizeof(length));
m_data.append(name);
m_lastTextSegmentLength = 0;
}

} // namespace trimja
6 changes: 3 additions & 3 deletions src/evalstring.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class EvalString {

private:
std::string m_data;
Offset m_lastSegmentLength;
Offset m_lastTextSegmentLength;

public:
/**
Expand Down Expand Up @@ -102,13 +102,13 @@ class EvalString {
/**
* @brief Appends text to the EvalString, consolidating consecutive text
* sections.
* @param text The text to append.
* @param text The text to append. This must not be empty.
*/
void appendText(std::string_view text);

/**
* @brief Appends a variable to the EvalString.
* @param name The name of the variable to append.
* @param name The name of the variable to append. This must not be empty.
*/
void appendVariable(std::string_view name);
};
Expand Down
2 changes: 1 addition & 1 deletion tests/variables/build.ninja
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pool bye
depth = 100
foo = 42
rule copy
command = ninja --version $in -> $out foo=$foo bar=${bar} pool=$pool
command = ninja --version $in -> $out foo=$foo bar=${bar}$bar pool=$pool
pool = hi
build out1: copy in1
foo = 1
Expand Down