Skip to content

WIP - Replace boost memory-mapped files with own implementation.#387

Closed
junhaoliao wants to merge 2 commits into
y-scope:mainfrom
junhaoliao:zstd-dec-memmap
Closed

WIP - Replace boost memory-mapped files with own implementation.#387
junhaoliao wants to merge 2 commits into
y-scope:mainfrom
junhaoliao:zstd-dec-memmap

Conversation

@junhaoliao

Copy link
Copy Markdown
Member

References

When compiling the Zstd decompressor code with Emscripten for use in Log Viewer WebUI, it was identified that the code has dependency on Boost's memory-mapped files. That requires setting up extra compiling procedures - downloading Boost and also making modifications in Boost to avoid emcc complaining on missing some std::atomic implementations. However, the Boost dependencies can be replace with POSIX system calls.

In Emscripten compiled example code, with the replacement the speedup is > 30 ms.
 

Description

This is WIP. Criticism is welcomed:

  1. Shall we abstract the changes into a class to avoid bloating the Decoder class with 3 extra members?
  2. Shall we replace other Boost memory-mapped file usages in CLP core?

  1. Replace boost memory-mapped files with own implementation.

Validation performed

  1. Built and ran unitTest target and passed all:
    ...
    All tests passed (95962 assertions in 51 test cases)
    

Comment on lines +128 to +129
size_t m_compressed_file_size;
char* m_mem_mapped_compressed_file_buffer;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should use std::span (probably wrapping with std::optional) instead

@kirkrodrigues

Copy link
Copy Markdown
Member
  1. Shall we abstract the changes into a class to avoid bloating the Decoder class with 3 extra members?
  2. Shall we replace other Boost memory-mapped file usages in CLP core?

If we do (2), then we should do (1). I think it would be a good idea to do both, but if we're time constrained, I think we could go with the current approach. If so, we can convert this to non-WIP and do a proper review.

@LinZhihao-723

Copy link
Copy Markdown
Member

This PR has been formally implemented in #445

@junhaoliao junhaoliao deleted the zstd-dec-memmap branch May 7, 2026 19:46
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.

3 participants