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

zlib:Support using Kconfig to control memory usage in zlib #2734

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

anjiahao1
Copy link
Contributor

Summary

zlib:Support using Kconfig to control memory usage in zlib

Impact

zlib

Testing

qmeu with zlib

Signed-off-by: anjiahao <anjiahao@xiaomi.com>
@nuttxpr
Copy link

nuttxpr commented Oct 15, 2024

[Experimental Bot, please feedback here]

No, this PR does not meet the NuttX requirements.

Here's why and how to improve it:

Missing Information

  • Summary:
    • Why? What problem does this change solve? Why use Kconfig for this?
    • What/How? Be more specific. Which parts of zlib's memory usage are configurable? What are the Kconfig options?
  • Impact:
    • You've only listed "zlib" as impacted, but not how. Does this change add a new feature? Optimize existing behavior?
    • Address all impact points (user adaptation, build process, hardware, documentation, security, compatibility). Even if the answer is "NO", state it explicitly.
  • Testing:
    • Insufficient detail: "qemu with zlib" is not descriptive enough. Provide:
      • Specific QEMU machine type (e.g., qemu-system-arm -M versatilepb)
      • NuttX configuration used (boards/<arch>/<board>/configs/<config_name>.h)
      • Relevant log snippets: Only include the parts of the log that demonstrate the change's effect.

Example of an Improved PR Description

## Summary

This PR introduces Kconfig options to control zlib's memory usage, allowing users to tailor its footprint for memory-constrained systems. Specifically, it adds options for:

* Configuring the size of zlib's internal buffers.
* Enabling/disabling memory allocation strategies that trade off compression ratio for reduced memory consumption.

This change is motivated by [link to relevant NuttX issue, if any].

## Impact

* **Is new feature added?** YES - Fine-grained control over zlib's memory usage.
* **Impact on user?** YES - Users can now configure zlib's memory usage via Kconfig.  Documentation will be updated to describe the new options.
* **Impact on build?** NO
* **Impact on hardware?** NO 
* **Impact on documentation?** YES -  Documentation will be updated to describe the new Kconfig options and their impact on memory usage and compression performance.
* **Impact on security?** NO
* **Impact on compatibility?** NO
* **Anything else to consider?** N/A

## Testing

I confirm that changes are verified on the following setup:

* **Build Host:** Ubuntu 20.04, GCC 9.4.0
* **Target:** QEMU VersatilePB (ARM), `boards/arm/armv7-a/sim/configs/sim_smp.h`

**Testing logs before change:**

<relevant snippet showing zlib's memory usage without the Kconfig options>


**Testing logs after change:**

<relevant snippet showing zlib's memory usage with the new Kconfig options enabled>

By providing clear and comprehensive information, you make it easier for reviewers to understand and assess your PR.

@xiaoxiang781216 xiaoxiang781216 merged commit 8130d7e into apache:master Oct 16, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants