Skip to content

Commit

Permalink
Create a SpanSearchValue class that allows tree-searching without e…
Browse files Browse the repository at this point in the history
…xtra intermediate `if null/missing` checks. (#36754)

* Start adding the fluent tree object

* Start adding unit tests

* Add test file

* more testing

* update sizes hint and make use of things into CodegenDataModelProvider

* Restyle

* Fix some commments

* More merge fixes

* Remove some odd copy&paste comments

* Update src/lib/support/FluentTreeObject.h

Co-authored-by: Boris Zbarsky <[email protected]>

* Update src/lib/support/FluentTreeObject.h

Co-authored-by: Boris Zbarsky <[email protected]>

* Fix up comments a bit

* Simplify FluentTreeObject a bit

* Extra wrapper not needed

* Cleaned up comments

* Restyled by clang-format

* Sed rename FluentTreeObject to SpanSearchValue

* Also rename files

* Restyle

* Very slight reduction in complexity that reduces extra flash usage by a few bytes

* Another minor source code size decrease

* Even slightly smaller code

* Restyle

* Fix comment typo

* make cc32xx compile optimized for size by default, so we can treak flash usage as such

* Restyled by gn

* Update src/data-model-providers/codegen/CodegenDataModelProvider.cpp

Co-authored-by: Boris Zbarsky <[email protected]>

* Update src/lib/support/SpanSearchValue.h

Co-authored-by: Boris Zbarsky <[email protected]>

* Update src/lib/support/SpanSearchValue.h

Co-authored-by: Boris Zbarsky <[email protected]>

* Update src/lib/support/SpanSearchValue.h

Co-authored-by: Boris Zbarsky <[email protected]>

* Rename tree to searchable

* Fix comment

* Restyled by clang-format

---------

Co-authored-by: Andrei Litvin <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
4 people authored Dec 13, 2024
1 parent 9e203e2 commit c9a5d13
Show file tree
Hide file tree
Showing 6 changed files with 373 additions and 60 deletions.
87 changes: 27 additions & 60 deletions src/data-model-providers/codegen/CodegenDataModelProvider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include <lib/core/CHIPError.h>
#include <lib/core/DataModelTypes.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/SpanSearchValue.h>

#include <optional>
#include <variant>
Expand Down Expand Up @@ -106,6 +107,19 @@ using detail::EnumeratorCommandFinder;

namespace {

/// Search by device type within a span of EmberAfDeviceType (finds the device type that matches the given
/// DataModel::DeviceTypeEntry)
struct ByDeviceType
{
using Key = DataModel::DeviceTypeEntry;
using Type = const EmberAfDeviceType;
static Span<Type> GetSpan(Span<const EmberAfDeviceType> & data) { return data; }
static bool HasKey(const Key & id, const Type & instance)
{
return (instance.deviceId == id.deviceTypeId) && (instance.deviceVersion == id.deviceTypeRevision);
}
};

const CommandId * AcceptedCommands(const EmberAfCluster & cluster)
{
return cluster.acceptedCommandList;
Expand Down Expand Up @@ -268,51 +282,17 @@ DataModel::CommandEntry CommandEntryFrom(const ConcreteClusterPath & clusterPath
// to a common type is probably better. Need to figure out dependencies since
// this would make ember return datamodel-provider types.
// See: https://github.com/project-chip/connectedhomeip/issues/35889
DataModel::DeviceTypeEntry DeviceTypeEntryFromEmber(const EmberAfDeviceType & other)
std::optional<DataModel::DeviceTypeEntry> DeviceTypeEntryFromEmber(const EmberAfDeviceType * other)
{
DataModel::DeviceTypeEntry entry;

entry.deviceTypeId = other.deviceId;
entry.deviceTypeRevision = other.deviceVersion;

return entry;
}

// Explicitly compare for identical entries. note that types are different,
// so you must do `a == b` and the `b == a` will not work.
bool operator==(const DataModel::DeviceTypeEntry & a, const EmberAfDeviceType & b)
{
return (a.deviceTypeId == b.deviceId) && (a.deviceTypeRevision == b.deviceVersion);
}

/// Find the `index` where one of the following holds:
/// - types[index - 1] == previous OR
/// - index == types.size() // i.e. not found or there is no next
///
/// hintWherePreviousMayBe represents a search hint where previous may exist.
unsigned FindNextDeviceTypeIndex(Span<const EmberAfDeviceType> types, const DataModel::DeviceTypeEntry & previous,
unsigned hintWherePreviousMayBe)
{
if (hintWherePreviousMayBe < types.size())
{
// this is a valid hint ... see if we are lucky
if (previous == types[hintWherePreviousMayBe])
{
return hintWherePreviousMayBe + 1; // return the next index
}
}

// hint was not useful. We have to do a full search
for (unsigned idx = 0; idx < types.size(); idx++)
if (other == nullptr)
{
if (previous == types[idx])
{
return idx + 1;
}
return std::nullopt;
}

// cast should be safe as we know we do not have that many types
return static_cast<unsigned>(types.size());
return DataModel::DeviceTypeEntry{
.deviceTypeId = other->deviceId,
.deviceTypeRevision = other->deviceVersion,
};
}

const ConcreteCommandPath kInvalidCommandPath(kInvalidEndpointId, kInvalidClusterId, kInvalidCommandId);
Expand Down Expand Up @@ -894,15 +874,9 @@ std::optional<DataModel::DeviceTypeEntry> CodegenDataModelProvider::FirstDeviceT

CHIP_ERROR err = CHIP_NO_ERROR;
Span<const EmberAfDeviceType> deviceTypes = emberAfDeviceTypeListFromEndpointIndex(*endpoint_index, err);
SpanSearchValue<chip::Span<const EmberAfDeviceType>> searchable(&deviceTypes);

if (deviceTypes.empty())
{
return std::nullopt;
}

// we start at the beginning
mDeviceTypeIterationHint = 0;
return DeviceTypeEntryFromEmber(deviceTypes[0]);
return DeviceTypeEntryFromEmber(searchable.First<ByDeviceType>(mDeviceTypeIterationHint).Value());
}

std::optional<DataModel::DeviceTypeEntry> CodegenDataModelProvider::NextDeviceType(EndpointId endpoint,
Expand All @@ -917,18 +891,11 @@ std::optional<DataModel::DeviceTypeEntry> CodegenDataModelProvider::NextDeviceTy
return std::nullopt;
}

CHIP_ERROR err = CHIP_NO_ERROR;
Span<const EmberAfDeviceType> deviceTypes = emberAfDeviceTypeListFromEndpointIndex(*endpoint_index, err);

unsigned idx = FindNextDeviceTypeIndex(deviceTypes, previous, mDeviceTypeIterationHint);

if (idx >= deviceTypes.size())
{
return std::nullopt;
}
CHIP_ERROR err = CHIP_NO_ERROR;
chip::Span<const EmberAfDeviceType> deviceTypes = emberAfDeviceTypeListFromEndpointIndex(*endpoint_index, err);
SpanSearchValue<chip::Span<const EmberAfDeviceType>> searchable(&deviceTypes);

mDeviceTypeIterationHint = idx;
return DeviceTypeEntryFromEmber(deviceTypes[idx]);
return DeviceTypeEntryFromEmber(searchable.Next<ByDeviceType>(previous, mDeviceTypeIterationHint).Value());
}

std::optional<DataModel::Provider::SemanticTag> CodegenDataModelProvider::GetFirstSemanticTag(EndpointId endpoint)
Expand Down
1 change: 1 addition & 0 deletions src/lib/support/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ static_library("support") {
"ScopedBuffer.h",
"SetupDiscriminator.h",
"SortUtils.h",
"SpanSearchValue.h",
"StateMachine.h",
"StringBuilder.cpp",
"StringBuilder.h",
Expand Down
154 changes: 154 additions & 0 deletions src/lib/support/SpanSearchValue.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
/*
* Copyright (c) 2024 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#pragma once

#include <cstddef>
#include <lib/support/Span.h>

#include <optional>

namespace chip {

/// represents a wrapper around a type `T` that contains internal
/// `Span<...>` values of other sub-types. It allows searching within the container sub-spans
/// to create new containers.
///
/// The use case is that we very often search within nested containers, like "find-endpoint" + "find-cluster" + "find-attribute"
/// and we generally only care about "does the last element exist or not"
///
/// A typical example of the way this class is used looks like this:
///
/// SpanSearchValue container(somePointer);
///
/// const AcceptedCommandData * value =
/// container
/// .Find<ByEndpoint>(path.mEndpointId, mEndpointIndexHint)
/// .Find<ByServerCluster>(path.mClusterId, mServerClusterHint)
/// .Find<ByAcceptedCommand>(path.mCommandId, mAcceptedCommandHint)
/// .Value();
///
/// Where a `ByFoo` structure looks like:
///
/// struct ByFoo {
/// using Key = int; // the KEY inside a type
/// using Type = SomeValueType; // The type that is indexed by `Key`
///
/// /// Allows getting the "Span of Type" from an underlying structure.
/// /// A `SpanSearchValue<Foo>` will require a `GetSpan(Foo&)`
/// static Span<Type> GetSpan(ContainerType & data) { /* return ... */ }
///
/// /// Checks that the `Type` value has the given `Key` or not
/// static bool HasKey(const Key & id, const Type & instance) { /* return "instance has key id" */ }
/// }
///
/// Where we define:
/// - how to get a "span of sub-elements" for an object (`GetSpan`)
/// - how to determine if a given sub-element has the "correct key"
template <typename T>
class SpanSearchValue
{
public:
SpanSearchValue() : mValue(nullptr) {}
SpanSearchValue(std::nullptr_t) : mValue(nullptr) {}
explicit SpanSearchValue(T * value) : mValue(value) {}

/// Returns nullptr if such an element does not exist or non-null valid value if the element exists
T * Value() const { return mValue; }

/// Gets the first element of `TYPE::Type`
template <typename TYPE>
SpanSearchValue<typename TYPE::Type> First(unsigned & indexHint)
{
// if no value, searching more also yields no value
VerifyOrReturnValue(mValue != nullptr, nullptr);

Span<typename TYPE::Type> value_span = TYPE::GetSpan(*mValue);
VerifyOrReturnValue(!value_span.empty(), nullptr);

// found it, save the hint
indexHint = 0;
return SpanSearchValue<typename TYPE::Type>(&value_span[0]);
}

/// Find the value corresponding to `key`
template <typename TYPE>
SpanSearchValue<typename TYPE::Type> Find(typename TYPE::Key key, unsigned & indexHint)
{
VerifyOrReturnValue(mValue != nullptr, nullptr);

Span<typename TYPE::Type> value_span = TYPE::GetSpan(*mValue);

if (!FindIndexUsingHint(key, value_span, indexHint, TYPE::HasKey))
{
return nullptr;
}

return SpanSearchValue<typename TYPE::Type>(&value_span[indexHint]);
}

/// Finds the value that occurs after `key` in the underlying collection.
template <typename TYPE>
SpanSearchValue<typename TYPE::Type> Next(typename TYPE::Key key, unsigned & indexHint)
{
VerifyOrReturnValue(mValue != nullptr, nullptr);

Span<typename TYPE::Type> value_span = TYPE::GetSpan(*mValue);

if (!FindIndexUsingHint(key, value_span, indexHint, TYPE::HasKey))
{
return nullptr;
}

VerifyOrReturnValue((indexHint + 1) < value_span.size(), nullptr);

indexHint++;
return SpanSearchValue<typename TYPE::Type>(&value_span[indexHint]);
}

private:
T * mValue = nullptr; // underlying value, NULL if such a value does not exist

/// Search for the index where `needle` is located inside `haystack`
///
/// using `haystackValueMatchesNeedle` to find if a given haystack value matches the given needle
///
/// `in_out_hint` contains a start search point at the start and will contain the found index
/// location (if found) at the end.
///
/// Returns true on success (index found) false on failure (index not found). If returning
/// false, the value of `in_out_hint` is unchanged
template <typename N, typename H>
static bool FindIndexUsingHint(const N & needle, Span<H> haystack, unsigned & in_out_hint,
bool (*haystackValueMatchesNeedle)(const N &, const typename std::remove_const<H>::type &))
{
// search starts at `hint` rather than 0
const unsigned haystackSize = static_cast<unsigned>(haystack.size());
unsigned checkIndex = (in_out_hint < haystackSize) ? in_out_hint : 0;

for (unsigned i = 0; i < haystackSize; i++, checkIndex++)
{
if (haystackValueMatchesNeedle(needle, haystack[checkIndex % haystackSize]))
{
in_out_hint = checkIndex % haystackSize;
return true;
}
}

return false;
}
};

} // namespace chip
1 change: 1 addition & 0 deletions src/lib/support/tests/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ chip_test_suite("tests") {
"TestScopedBuffer.cpp",
"TestSorting.cpp",
"TestSpan.cpp",
"TestSpanSearchValue.cpp",
"TestStateMachine.cpp",
"TestStaticSupportSmartPtr.cpp",
"TestStringBuilder.cpp",
Expand Down
Loading

0 comments on commit c9a5d13

Please sign in to comment.