Skip to content

Commit

Permalink
Save flash on generated struct encoding logic (#29608)
Browse files Browse the repository at this point in the history
* Save flash on generated struct encoding logic

Problem:

- Structural repetition in the generated code caused needless
  jumps to be inlined in very large numbers in `Encode` methods,
  in ways that could be factored-out.

This PR:

- Changes the generated code to use a proxy encoder class that
  reduces expanded inlined code, reduces 64 bit arguments usage
  and maintains correct early return behavior, while generating
  significantly smaller method bodies.

Testing done:

- Unit tests pass
- Integration tests pass

* Restyled by clang-format

* Address review comments

* Update src/app/data-model/WrappedStructEncoder.h

Co-authored-by: Damian Królik <[email protected]>

* Regen

---------

Co-authored-by: [email protected] <[email protected]>
Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Damian Królik <[email protected]>
  • Loading branch information
4 people authored Oct 6, 2023
1 parent 43981ac commit fc44549
Show file tree
Hide file tree
Showing 4 changed files with 1,786 additions and 2,072 deletions.
64 changes: 64 additions & 0 deletions src/app/data-model/WrappedStructEncoder.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright (c) 2023 Project CHIP Authors
* All rights reserved.
*
* 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 <app/data-model/Encode.h>

#include <lib/core/CHIPError.h>
#include <lib/core/DataModelTypes.h>
#include <lib/core/TLV.h>

#include <utility>

namespace chip {
namespace app {
namespace DataModel {

class WrappedStructEncoder
{
public:
WrappedStructEncoder(TLV::TLVWriter & writer, TLV::Tag outerTag) : mWriter(writer)
{
mLastError = mWriter.StartContainer(outerTag, TLV::kTLVType_Structure, mOuter);
}

template <typename... Args>
void Encode(uint8_t contextTag, Args &&... args)
{
VerifyOrReturn(mLastError == CHIP_NO_ERROR);
mLastError = DataModel::Encode(mWriter, TLV::ContextTag(contextTag), std::forward<Args>(args)...);
}

CHIP_ERROR Finalize()
{
if (mLastError == CHIP_NO_ERROR)
{
mLastError = mWriter.EndContainer(mOuter);
}
return mLastError;
}

private:
TLV::TLVWriter & mWriter;
CHIP_ERROR mLastError = CHIP_NO_ERROR;
TLV::TLVType mOuter;
};

} // namespace DataModel
} // namespace app
} // namespace chip
24 changes: 12 additions & 12 deletions src/app/zap-templates/partials/cluster-objects-struct.zapt
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ namespace {{asUpperCamelCase name}} {

} // namespace {{asUpperCamelCase name}}
{{else}}

namespace {{asUpperCamelCase name}} {
{{#if isFabricScoped}}
CHIP_ERROR Type::EncodeForWrite(TLV::TLVWriter & aWriter, TLV::Tag aTag) const
Expand All @@ -81,34 +82,33 @@ CHIP_ERROR Type::DoEncode(TLV::TLVWriter & aWriter, TLV::Tag aTag, const Optiona
{{#if struct_has_fabric_sensitive_fields}}
bool includeSensitive = !aAccessingFabricIndex.HasValue() || (aAccessingFabricIndex.Value() == fabricIndex);
{{/if}}
TLV::TLVType outer;
ReturnErrorOnFailure(aWriter.StartContainer(aTag, TLV::kTLVType_Structure, outer));

DataModel::WrappedStructEncoder encoder{aWriter, aTag};

{{#zcl_struct_items}}
{{#if (is_num_equal fieldIdentifier 254)}}
if (aAccessingFabricIndex.HasValue()) {
ReturnErrorOnFailure(DataModel::Encode(aWriter, TLV::ContextTag(Fields::k{{asUpperCamelCase label}}), {{asLowerCamelCase label}}));
encoder.Encode(to_underlying(Fields::k{{asUpperCamelCase label}}), {{asLowerCamelCase label}});
}
{{else if isFabricSensitive}}
if (includeSensitive) {
ReturnErrorOnFailure(DataModel::Encode(aWriter, TLV::ContextTag(Fields::k{{asUpperCamelCase label}}), {{asLowerCamelCase label}}));
encoder.Encode(to_underlying(Fields::k{{asUpperCamelCase label}}), {{asLowerCamelCase label}});
}
{{else}}
ReturnErrorOnFailure(DataModel::Encode(aWriter, TLV::ContextTag(Fields::k{{asUpperCamelCase label}}), {{asLowerCamelCase label}}));
encoder.Encode(to_underlying(Fields::k{{asUpperCamelCase label}}), {{asLowerCamelCase label}});
{{/if}}
{{/zcl_struct_items}}
ReturnErrorOnFailure(aWriter.EndContainer(outer));
return CHIP_NO_ERROR;

return encoder.Finalize();
}
{{else}}
CHIP_ERROR Type::Encode(TLV::TLVWriter & aWriter, TLV::Tag aTag) const
{
TLV::TLVType outer;
ReturnErrorOnFailure(aWriter.StartContainer(aTag, TLV::kTLVType_Structure, outer));
DataModel::WrappedStructEncoder encoder{aWriter, aTag};
{{#zcl_struct_items}}
ReturnErrorOnFailure(DataModel::Encode(aWriter, TLV::ContextTag(Fields::k{{asUpperCamelCase label}}), {{asLowerCamelCase label}}));
encoder.Encode(to_underlying(Fields::k{{asUpperCamelCase label}}), {{asLowerCamelCase label}});
{{/zcl_struct_items}}
ReturnErrorOnFailure(aWriter.EndContainer(outer));
return CHIP_NO_ERROR;
return encoder.Finalize();
}
{{/if}}

Expand Down
9 changes: 5 additions & 4 deletions src/app/zap-templates/templates/app/cluster-objects-src.zapt
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
{{> header}}

#include <app/data-model/WrappedStructEncoder.h>
#include <app-common/zap-generated/cluster-objects.h>

#include <variant>

namespace chip {
Expand Down Expand Up @@ -80,12 +82,11 @@ namespace Commands {
{{#zcl_commands}}
namespace {{asUpperCamelCase name}} {
CHIP_ERROR Type::Encode(TLV::TLVWriter & aWriter, TLV::Tag aTag) const{
TLV::TLVType outer;
ReturnErrorOnFailure(aWriter.StartContainer(aTag, TLV::kTLVType_Structure, outer));
DataModel::WrappedStructEncoder encoder{aWriter, aTag};
{{#zcl_command_arguments}}
ReturnErrorOnFailure(DataModel::Encode(aWriter, TLV::ContextTag(Fields::k{{asUpperCamelCase label}}), {{asLowerCamelCase label}}));
encoder.Encode(to_underlying(Fields::k{{asUpperCamelCase label}}), {{asLowerCamelCase label}});
{{/zcl_command_arguments}}
return aWriter.EndContainer(outer);
return encoder.Finalize();
}

CHIP_ERROR DecodableType::Decode(TLV::TLVReader &reader) {
Expand Down
Loading

0 comments on commit fc44549

Please sign in to comment.