Cycles: Compress GPU kernels to reduce file size #123557

Merged
Lukas Stockner merged 5 commits from LukasStockner/blender:cycles-kernel-zstd into blender-v4.2-release 2024-06-23 00:52:39 +02:00
Member

Precompiled Cycles kernels make up a considerable fraction of the total size of
Blender builds nowadays. As we add more features and support for more
architectures, this will only continue to increase.

However, since these kernels tend to be quite compressible, we can save a lot
of storage by storing them in compressed form and decompressing the required
kernel(s) during loading.

By using Zstandard compression with a high level, we can get decent compression
ratios (~5x for the current kernels) while keeping decompression time low
(about 30ms in the worse case in my tests). And since we already require zstd
for Blender, this doesn't introduce a new dependency.

While the main improvement is to the size of the extracted Blender installation
(which is reduced by ~400-500MB currently), this also shrinks the download on
Windows, since .zip's deflate compression is less effective. It doesn't help on
Linux since we're already using .tar.xz there, but the smaller installed size
is still a good thing.

See #123522 for initial discussion.

Precompiled Cycles kernels make up a considerable fraction of the total size of Blender builds nowadays. As we add more features and support for more architectures, this will only continue to increase. However, since these kernels tend to be quite compressible, we can save a lot of storage by storing them in compressed form and decompressing the required kernel(s) during loading. By using Zstandard compression with a high level, we can get decent compression ratios (~5x for the current kernels) while keeping decompression time low (about 30ms in the worse case in my tests). And since we already require zstd for Blender, this doesn't introduce a new dependency. While the main improvement is to the size of the extracted Blender installation (which is reduced by ~400-500MB currently), this also shrinks the download on Windows, since .zip's deflate compression is less effective. It doesn't help on Linux since we're already using .tar.xz there, but the smaller installed size is still a good thing. See #123522 for initial discussion.
Lukas Stockner added this to the 4.2 LTS milestone 2024-06-21 14:56:45 +02:00
Lukas Stockner added 1 commit 2024-06-21 14:56:52 +02:00
Cycles: Compress GPU kernels to reduce file size
Some checks failed
buildbot/vexp-code-patch-lint Build done.
buildbot/vexp-code-patch-linux-x86_64 Build done.
buildbot/vexp-code-patch-darwin-x86_64 Build done.
buildbot/vexp-code-patch-darwin-arm64 Build done.
buildbot/vexp-code-patch-windows-amd64 Build done.
buildbot/vexp-code-patch-coordinator Build done.
f9c9c40321
Lukas Stockner added this to the Render & Cycles project 2024-06-21 14:56:59 +02:00
Lukas Stockner requested review from Brecht Van Lommel 2024-06-21 14:57:08 +02:00
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR123557) when ready.
Lukas Stockner added 2 commits 2024-06-21 15:02:24 +02:00
Also compress HIP(RT)
Some checks failed
buildbot/vexp-code-patch-lint Build done.
buildbot/vexp-code-patch-darwin-x86_64 Build done.
buildbot/vexp-code-patch-darwin-arm64 Build done.
buildbot/vexp-code-patch-linux-x86_64 Build done.
buildbot/vexp-code-patch-windows-amd64 Build done.
buildbot/vexp-code-patch-coordinator Build done.
f7d24fed6c

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR123557) when ready.
Brecht Van Lommel requested changes 2024-06-21 15:39:36 +02:00
Dismissed
Brecht Van Lommel left a comment
Owner

That was really quick. I only have one minor comment.

That was really quick. I only have one minor comment.
@ -0,0 +20,4 @@
}
long in_size = ftell(in);
if (in_size < 0) {
return -1;

It maybe doesn't matter in practice, but I would prefer to fclose(in) and fclose(out) on these types of errors anyway. Maybe at some point it trips up a tool that checks for leaked file handles.

It maybe doesn't matter in practice, but I would prefer to `fclose(in)` and `fclose(out)` on these types of errors anyway. Maybe at some point it trips up a tool that checks for leaked file handles.
brecht marked this conversation as resolved
Brecht Van Lommel requested review from Sergey Sharybin 2024-06-21 15:40:02 +02:00

Adding @Sergey as reviewer, mainly to check if he agrees this is ok to go into 4.2.

Adding @Sergey as reviewer, mainly to check if he agrees this is ok to go into 4.2.

Would be good to note changes percentage in description.

Would be good to note changes percentage in description.
Member

Should this be behind a cmake option? I could see during development you may want to skip the endless compress/decompress cycle

Should this be behind a cmake option? I could see during development you may want to skip the endless compress/decompress cycle

Should this be behind a cmake option? I could see during development you may want to skip the endless compress/decompress cycle

I think compression time is negligible compared to kernel compilation time. And decompression should be near instant for users, so should so also be ok for developers.

The only reason I'd have this as an option would be to make the zstd dependency optional, but it's already required for blend file compression.

We could change path_read_binary and path_read_text to automatically work with compressed files by checking for the existence of a file with .zst extension, and if not trying to read it without that extension. That makes the implementation a bit more elegant and easier to make it optional if we'd want that.

> Should this be behind a cmake option? I could see during development you may want to skip the endless compress/decompress cycle I think compression time is negligible compared to kernel compilation time. And decompression should be near instant for users, so should so also be ok for developers. The only reason I'd have this as an option would be to make the zstd dependency optional, but it's already required for blend file compression. We could change `path_read_binary` and `path_read_text` to automatically work with compressed files by checking for the existence of a file with `.zst` extension, and if not trying to read it without that extension. That makes the implementation a bit more elegant and easier to make it optional if we'd want that.
Author
Member

We could change path_read_binary and path_read_text to automatically work with compressed files by checking for the existence of a file with .zst extension, and if not trying to read it without that extension. That makes the implementation a bit more elegant and easier to make it optional if we'd want that.

I had originally done that, but it makes the code more complex since we also need to adapt the path_exists checks.
I guess we could add an optional "check_compressed" flag to path_exists and path_read_*.

> We could change `path_read_binary` and `path_read_text` to automatically work with compressed files by checking for the existence of a file with `.zst` extension, and if not trying to read it without that extension. That makes the implementation a bit more elegant and easier to make it optional if we'd want that. I had originally done that, but it makes the code more complex since we also need to adapt the path_exists checks. I guess we could add an optional "check_compressed" flag to path_exists and path_read_*.

I had originally done that, but it makes the code more complex since we also need to adapt the path_exists checks.
I guess we could add an optional "check_compressed" flag to path_exists and path_read_*.

Ah right. That's arguably more complex, so I would not bother then.

> I had originally done that, but it makes the code more complex since we also need to adapt the path_exists checks. > I guess we could add an optional "check_compressed" flag to path_exists and path_read_*. Ah right. That's arguably more complex, so I would not bother then.
Brecht Van Lommel requested changes 2024-06-21 16:43:13 +02:00
Dismissed
Brecht Van Lommel left a comment
Owner

Checking the build log, seems it's not installing .zst files for .fatbin and hipfb.

Checking the build log, seems it's not installing `.zst` files for `.fatbin` and `hipfb`.
Author
Member

Checking the build log, seems it's not installing .zst files for .fatbin and hipfb.

In the ongoing build? My initial commit was missing those, but now they should be included.

> Checking the build log, seems it's not installing `.zst` files for `.fatbin` and `hipfb`. In the ongoing build? My initial commit was missing those, but now they should be included.
Brecht Van Lommel requested changes 2024-06-21 17:22:40 +02:00
Dismissed
@ -0,0 +1,57 @@
#include <cstdint>

Add SPDX copyright/license header.

Add SPDX copyright/license header.
brecht marked this conversation as resolved

In the ongoing build? My initial commit was missing those, but now they should be included.

Ah right, it seems I was looking at an old build log.

> In the ongoing build? My initial commit was missing those, but now they should be included. Ah right, it seems I was looking at an old build log.
Sergey Sharybin reviewed 2024-06-21 17:27:59 +02:00
Sergey Sharybin left a comment
Owner

If this solves an issue with pypi then it is fine to target this for 4.2.

I'd suggest having commit message more self-container, with main outlines of motivation. For the details is can still reference the design task.

If this solves an issue with pypi then it is fine to target this for 4.2. I'd suggest having commit message more self-container, with main outlines of motivation. For the details is can still reference the design task.
@ -0,0 +10,4 @@
return -1;
}
FILE *in = fopen(argv[1], "rb");

This has a potential of failing with non-ascii paths on Windows. But on a bigger picture I don't think it will be a problem in reality: CMake itself had issues with non-ascii paths last time i've been looking into it.

So more of a FYI, or maybe worth a TODO.

This has a potential of failing with non-ascii paths on Windows. But on a bigger picture I don't think it will be a problem in reality: CMake itself had issues with non-ascii paths last time i've been looking into it. So more of a FYI, or maybe worth a TODO.

This seems about as expected.

Before  After
Windows .zip 458MB 344MB
Windows extracted 1365MB 854MB
Linux .tar.xz 313MB 317MB
Linux extracted 1594MB 1191MB
This seems about as expected. | | Before | After | |-|-|-| | Windows .zip | 458MB | 344MB | | Windows extracted | 1365MB | 854MB | | Linux .tar.xz | 313MB | 317MB | | Linux extracted | 1594MB | 1191MB |
Lukas Stockner added 1 commit 2024-06-22 17:22:02 +02:00
Convert compressor tool to C++ IO
Some checks failed
buildbot/vexp-code-patch-lint Build done.
buildbot/vexp-code-patch-darwin-arm64 Build done.
buildbot/vexp-code-patch-darwin-x86_64 Build done.
buildbot/vexp-code-patch-linux-x86_64 Build done.
buildbot/vexp-code-patch-windows-amd64 Build done.
buildbot/vexp-code-patch-coordinator Build done.
b125eeec1a
Author
Member

Update:

  • Wrote proper commit message
  • Converted compressor tool to C++ IO to get RAII instead of copy-pasting fclose over and over.
  • Added SPDX header.

@blender-bot package

Update: - Wrote proper commit message - Converted compressor tool to C++ IO to get RAII instead of copy-pasting `fclose` over and over. - Added SPDX header. @blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR123557) when ready.
Brecht Van Lommel approved these changes 2024-06-22 23:00:31 +02:00
Brecht Van Lommel left a comment
Owner

There's a clang-format thing to fix, but that doesn't need re-review.

There's a clang-format thing to fix, but that doesn't need re-review.
Lukas Stockner added 1 commit 2024-06-23 00:52:14 +02:00
Lukas Stockner merged commit 4bde68cdd6 into blender-v4.2-release 2024-06-23 00:52:39 +02:00
Lukas Stockner deleted branch cycles-kernel-zstd 2024-06-23 00:52:43 +02:00
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Code Documentation
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No Assignees
6 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#123557
No description provided.