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 docker images, add musllinux build image and ccache support #273

Merged
merged 3 commits into from
Jan 9, 2024

Conversation

hendrikmuhs
Copy link
Contributor

@hendrikmuhs hendrikmuhs commented Jan 8, 2024

update docker files, more use of parallel building, add ccache and support of musllinux

Note: There seems to be a problem with the manylinux on arm image. For some reason snappy isn't found although it gets installed.

FWIW There was a problem with the boost download(server unavailable), I added support for a fallback.

@hendrikmuhs
Copy link
Contributor Author

I think I found the problem, hiredis-devel and libzstd-devel are not available for aarch64, that's why the whole line failed. The 2 are only necessary for ccache, but that doesn't built on manylinux for aarch64 anyway.

@hendrikmuhs hendrikmuhs mentioned this pull request Jan 8, 2024
Copy link
Member

@narekgharibyan narekgharibyan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much speed improvement we gain by using ccache here ?

@hendrikmuhs
Copy link
Contributor Author

The ccache support is only available together with #269. The ccache effect is available in the logs, e.g.

  /usr/local/bin/ccache -s
  Cacheable calls:     6 /  16 (37.50%)
    Hits:              5 /   6 (83.33%)
      Direct:          4 /   5 (80.00%)
      Preprocessed:    1 /   5 (20.00%)
    Misses:            1 /   6 (16.67%)
  Uncacheable calls:  10 /  16 (62.50%)
  Local storage:
    Cache size (GB): 0.0 / 0.5 ( 1.81%)
    Hits:              5 /   6 (83.33%)
    Misses:            1 /   6 (16.67%)
Save cache using key "ccache-macos-12-x86_64-python-2024-01-08T21:30:18.085Z".
/usr/local/bin/gtar --posix -cf cache.tzst --exclude cache.tzst -P -C /Users/runner/work/keyvi/keyvi --files-from manifest.txt --delay-directory-restore --use-compress-program zstdmt
Cache Size: ~8 MB (8075356 B)
Cache saved successfully

I admit, this only tells us that the cache did something, but not the gain. However the numbers should go up, I haven't enabled the other python versions yet. More in #269

@narekgharibyan
Copy link
Member

I guess it might have been easier to review/merge if we separate changes into smaller parts if possible at all. Like: -

  1. PR that fixes the docker images
  2. PR that adds muslin
  3. PR that adds cache

But this is just a suggestion :)

@hendrikmuhs hendrikmuhs merged commit cdd1787 into KeyviDev:master Jan 9, 2024
25 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.

2 participants