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

Update miniz to 2.1.0 #500

Closed
wants to merge 1 commit into from
Closed

Conversation

paravoid
Copy link
Contributor

@paravoid paravoid commented Jan 6, 2020

Update miniz to the latest version, 2.1.0. This is the stock, amalgated
miniz, with a few modifications, forward-ported from the existing diff
against 1.5:

  • Configuration options (the definitions at the top of miniz.h)
  • Disabling assert(), i.e. defining MZ_ASSERT to nothing
  • Defining TDEFL_LESS_MEMORY to 1
  • Setting TDEFL_LZ_DICT_SIZE = 8*1024 and TDEFL_LZ_CODE_BUF_SIZE = 8 * 1024

An additional complication is that the new miniz performs a long long to
integer division, which requires code from libgcc (__ashldi3), which we
didn't link to by default (due to -nostdlib). Add -lgcc to LDLIBS.

This seems to be a small increase in .text (8018->8023) with the stock
toolchain, but with a newer one (gcc 9.2.1/picolibc 1.3) actually
results in a decrease (7990->7947).

Tested on real hardware (D1 Mini Pro).

See #499 for some additional context.

Update miniz to the latest version, 2.1.0. This is the stock, amalgated
miniz, with a few modifications, forward-ported from the existing diff
against 1.5:
 * Configuration options (the definitions at the top of miniz.h)
 * Disabling assert(), i.e. defining MZ_ASSERT to nothing
 * Defining TDEFL_LESS_MEMORY to 1
 * Setting TDEFL_LZ_DICT_SIZE = 8*1024 and TDEFL_LZ_CODE_BUF_SIZE = 8 * 1024

An additional complication is that the new miniz performs a long long to
integer division, which requires code from libgcc (__ashldi3), which we
didn't link to by default (due to -nostdlib). Add -lgcc to LDLIBS.

This seems to be a small increase in .text (8018->8023) with the stock
toolchain, but with a newer one (gcc 9.2.1/picolibc 1.3) actually
results in a decrease (7990->7947).

Tested on real hardware (D1 Mini Pro).

See espressif#499 for some additional context.
@projectgus
Copy link
Contributor

Hi @paravoid ,

Thanks for this, I hadn't actually noticed that miniz development was continuing on GitHub.

I've cherry-picked your commit into our internal review & merge queue, this PR will be updated when it merges.

Angus

@paravoid
Copy link
Contributor Author

Thanks!

On that note, a two-parter idea to ease future migrations to newer versions:

  • It'd be useful to expose the changes changes to separate config options: expose TDEFL_LESS_MEMORY as MINIZ_LESS_MEMORY, and maybe the DICT_SIZE/CODE_BUF_SIZE as MINIZ_EVEN_LESS_MEMORY or something? I don't feel confident enough to support those dict size changes and such, so perhaps it would make sense for you or someone this project to raise that with miniz upstream?
  • I filed Split configuration into a separate header file richgel999/miniz#145 to ask for the config to be split into a different header file. Hopefully that could allow us to commit an esptool-specific miniz_config.h separately in this project, and then just carry a pristine miniz.h/c without code changes in this project.

antmak pushed a commit that referenced this pull request Jan 15, 2020
Update miniz to the latest version, 2.1.0. This is the stock, amalgated
miniz, with a few modifications, forward-ported from the existing diff
against 1.5:
 * Configuration options (the definitions at the top of miniz.h)
 * Disabling assert(), i.e. defining MZ_ASSERT to nothing
 * Defining TDEFL_LESS_MEMORY to 1
 * Setting TDEFL_LZ_DICT_SIZE = 8*1024 and TDEFL_LZ_CODE_BUF_SIZE = 8 * 1024

An additional complication is that the new miniz performs a long long to
integer division, which requires code from libgcc (__ashldi3), which we
didn't link to by default (due to -nostdlib). Add -lgcc to LDLIBS.

This seems to be a small increase in .text (8018->8023) with the stock
toolchain, but with a newer one (gcc 9.2.1/picolibc 1.3) actually
results in a decrease (7990->7947).

Tested on real hardware (D1 Mini Pro).

See #499 for some additional context.

Merges #500
@projectgus
Copy link
Contributor

Cherry-picked as 18647f2, thanks @paravoid :)

@projectgus projectgus closed this Jan 15, 2020
@projectgus
Copy link
Contributor

@paravoid Just saw your suggestions. Agree these sound good, making miniz into a submodule or something would be a nice cleanup. Will see what I can do.

dobairoland pushed a commit to espressif/esptool-legacy-flasher-stub that referenced this pull request May 30, 2024
Update miniz to the latest version, 2.1.0. This is the stock, amalgated
miniz, with a few modifications, forward-ported from the existing diff
against 1.5:
 * Configuration options (the definitions at the top of miniz.h)
 * Disabling assert(), i.e. defining MZ_ASSERT to nothing
 * Defining TDEFL_LESS_MEMORY to 1
 * Setting TDEFL_LZ_DICT_SIZE = 8*1024 and TDEFL_LZ_CODE_BUF_SIZE = 8 * 1024

An additional complication is that the new miniz performs a long long to
integer division, which requires code from libgcc (__ashldi3), which we
didn't link to by default (due to -nostdlib). Add -lgcc to LDLIBS.

This seems to be a small increase in .text (8018->8023) with the stock
toolchain, but with a newer one (gcc 9.2.1/picolibc 1.3) actually
results in a decrease (7990->7947).

Tested on real hardware (D1 Mini Pro).

See #499 for some additional context.

Merges espressif/esptool#500
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