-
Notifications
You must be signed in to change notification settings - Fork 82
Add MCCP4 support - New protocol created using zstd with substantial performance improvements. #98
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
base: master
Are you sure you want to change the base?
Conversation
Implemented support for MCCP4 (Zstandard) compression in the Phase_WILL function, allowing the client to negotiate this compression method. Updated logic to prevent agreeing to multiple compression methods simultaneously and added handling in the Phase_SUBNEGOTIATION_IAC function.
Refactor the compression logic to differentiate between MCCP1/MCCP2 (zlib) and MCCP4 (Zstandard). Implement direct processing for Zstandard compressed data and ensure proper cleanup of resources. Update statistics and error handling for both compression types.
Implement the core functionality for MCCP4 compression, including initialization, cleanup, and processing of Zstandard compressed data. This addition enhances the compression capabilities of MUSHclient, providing improved performance and better compression ratios compared to previous methods.
…9, to follow the standard progression (mccp = 85, mccp2 = 86, mccp3 = 87, mccp4 = 88)
…(Zstandard) support.
… and update statistics for MCCP4 (Zstandard).
This commit introduces a comprehensive set of new files for Zstandard compression support, including error handling, memory management, and decompression logic. Key components include zstd_errors.h, zstd.h, and various common utilities for efficient data processing. This enhancement lays the groundwork for improved compression capabilities using zstd.
This update improves the handling of decompressed data by ensuring accurate timing and memory management. It introduces a flag to indicate the end of a Zstandard frame, allowing for better processing of remaining raw Telnet bytes.
… function This update moves the timing measurement to occur after decompression but before I/O and display processing, ensuring more accurate tracking of compression time taken.
|
Not reviewed yet, but exploratory EXEs are available at https://github.com/Coffee-Nerd/mushclient/releases |
|
please include the license file(s) for zstd. Also include a readme for the zstd directory explaining exactly where it came from, what version it is, and how to update it. |
doc.cpp
Outdated
| // End timing here - after decompression but before I/O and display processing | ||
| if (App.m_iCounterFrequency) | ||
| { | ||
| QueryPerformanceCounter (&finish); | ||
| m_iCompressionTimeTaken += finish.QuadPart - start.QuadPart; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the point of moving this down from before the preceding error condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. This should be resolved with this
aae74b7
An additional timing stop has been added in case of error as well.
mccp4.cpp
Outdated
| // Cleanup on allocation failure | ||
| if (m_zstd_inbuf) free(m_zstd_inbuf); | ||
| if (m_zstd_outbuf) free(m_zstd_outbuf); | ||
| ZSTD_freeDStream((ZSTD_DStream*)m_zstd_dstream); | ||
| m_zstd_dstream = NULL; | ||
| m_zstd_inbuf = NULL; | ||
| m_zstd_outbuf = NULL; | ||
| m_zstd_incap = 0; | ||
| m_zstd_outcap = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks redundant with CleanupZstd below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very true. A call to CleanupZstd should be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be resolved with 92fcfd8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not without reducing this file down to a minimal change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait what? There were only a few line changes when I committed this.. I need to look at what happened.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be fixed with
6d5d62c
The line endings caused the issue with the major diff. It has also now been alphabetically sorted as requested, and the zstd header file included.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vcxproj diff needs to be trimmed way down.
It can be done manually from the original in a text editor.
- Add mccp4.cpp source file (alphabetically after mcdatetime.cpp)
- Add zstd library source files (after existing zlib files)
- Add zstd.h header file (alphabetically with other headers)
I also think you don't need all those zstd files, only some of them.
Fixed CRLF line endings in vcxproj file Removed unnecessary zstd files (pool.c, threading.c) Added only essential zstd decompression files Properly alphabetized file entries as requested
…taken, including stopping timing for early returns.
…Zstd function and improve timing accuracy during error scenarios.
pool.c/.h and threading.c/.h were also removed, seen here: |
…F-8 (with BOM) text, with CRLF line terminators
|
Update here: we're waiting on the gitattributes file to be merged so we can push the vcxproj file without the changed line-endings. |
… when compression is disabled and improve comments for clarity.
…y payloads, support multiple encodings, and improve error handling for Zstandard and deflate initialization. Update comments for better clarity.
…file as intended on push.
|
No objections here in principle, but do you have a document describing the protocol and a test mechanism, and is mudlet implementing it? |
Here are one of the latest drafts on MCCP4 (named MCCPX in draft, but it's MCCP4) https://www.last-outpost.com/LO/protocols/mccpx/draft-rahjiii-MCCPX-04.md Can also be found here: https://mudstandards.org/mud/mccpX_draft/ MCCP4 Mudlet implementation can be found here, but might be potentially not working as well as our implementation from what I've heard. They may have remedied that by now. Here is a test server I made with someone: You can also test MCCP4 (and many other things) against our client capabilities test server, found at test.mudvault.org 5050 The list of clients who have already implemented MCCP4 now are Lociterm, Mudlet, Crystal, MudForge (and MUSHclient, after this, which covers a HUGE percentage of all client users) If you want to test the client with a MUD, you can connect via darkwiz.org 6969 and play with the release .exe in the fork. |
|
Bump on this. |

Implements MCCP4 compression using Zstandard library as an alternative to existing MCCP2 (zlib).
Provides better compression ratios and performance while maintaining full backward compatibility.
Also moves the original timing for MCCP2 to not include WriteToLog or DisplayMsg in the timing, since those are i/o related and processing related and not related to compression timing.
Let me know if you have any questions, comments, concerns, or ideas.
Cheers.