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

Remove unused signature features #79

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Mike-M0
Copy link

@Mike-M0 Mike-M0 commented Jun 26, 2024

Describe your changes

  • only create xmldsig fragments which are listed (like other fragments)
  • remove unused fields for optimization (e.g remove "shall not be used" from standard)
    removed elements are not in structure and will result in en/decoder error NOT_IMPLEMENTED when "isUsed" flag is set or element is accessed in decoder.

Issue ticket number and link

Feature request #75

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have made corresponding changes to the documentation
  • I read the contribution documentation and made sure that my changes meet its requirements

- only create xmldsig framgnets which are listed (like other fragments)
- remove unused fields for optimization (per default remove "shall not be used" from standard)
removed elements are not in structure and will result in en/decoder error NOT_IMPLEMENTED when "isUsed" flag is set.

Signed-off-by: Michael Mezger <[email protected]>
@Mike-M0
Copy link
Author

Mike-M0 commented Aug 22, 2024

The change is not optimal.
I needed a manual change of eventCode in xmldsigFragment en/decoder from 1 to 32 and bitcount from 1 to 6 (which are the values from before this feature), because all other eventcodes are getting optimized out.
I could not find a easy way to implement this in generator.
But with this manual fix, PnC works fine again.

@barsnick barsnick changed the title Add feature from request #75: Remove unused signature features Oct 11, 2024
@barsnick
Copy link
Contributor

Notes for us when reviewing this:

  • Can this or should this be configurable (on/off)?
  • Does it affect the API?
  • Does it affect the ABI?
  • Is it free of regressions?

@barsnick
Copy link
Contributor

For the record, from my experiments:

  • All the elements disappear from the API.
  • Binary code size reduction (x86_64, standard generator options), iso-2 only: 1.7 to 2.1 % (reduced from 390272 to 383568 bytes stripped, 747752 to 731944 bytes unstripped)
  • Memory size reduction: from
    ISO-2 DC Struct sizes: exiDoc: 24184, V2G_Message: 24184, Header: 10360, Body: 13824
    to
    ISO-2 DC Struct sizes: exiDoc: 17064, V2G_Message: 17064, Header: 3240, Body: 13824

Our regression tests seem to pass.

@barsnick
Copy link
Contributor

If we take this in, we should also do the same optimizations for DIN and ISO 15118-20.

Applied to DIN and ISO -20:
HEAD @ d0b5310:

SAP Struct sizes:
	exiDoc: 588

DIN Struct sizes:
	exiDoc: 10976
	V2G_Message: 10976
	Header: 7976
	Body: 3000

ISO-2 DC Struct sizes:
	exiDoc: 24184
	V2G_Message: 24184
	Header: 10360
	Body: 13824

ISO-20 CommonMessages Struct sizes:
	exiDoc: 318392

ISO-20 AC Struct sizes:
	exiDoc: 10920

ISO-20 DC Struct sizes:
	exiDoc: 10840

With a modified version of this PR, rebased on HEAD:

SAP Struct sizes:
	exiDoc: 588

DIN Struct sizes:
	exiDoc: 4320
	V2G_Message: 4320
	Header: 1316
	Body: 3000

ISO-2 DC Struct sizes:
	exiDoc: 17064
	V2G_Message: 17064
	Header: 3240
	Body: 13824

ISO-20 CommonMessages Struct sizes:
	exiDoc: 311280

ISO-20 AC Struct sizes:
	exiDoc: 6000

ISO-20 DC Struct sizes:
	exiDoc: 6000

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants