Compress GAME_START message

[o01eg]

Alternative approach to #3854

It also breaks some compatibility but enforces only limited messages to be compressed.

[geoffthemedio]

are there plans to do something other than default_compression ?

[o01eg]

I suppose it possible to introduce some option.

[mlam19]

I'm working on porting over the user option and client-server compression usage requests from #3854. Since those work as bool flags, my message creation functions look like:

Message MakeMessage(const std::string& data, bool use_compression) {
    std::ostringstream os;
    {
        boost::iostreams::zlib_params params;
        params.level = boost::iostreams::zlib::no_compression;
        if (use_compression)
            params.level = boost::iostreams::zlib::default_compression;

        boost::iostreams::filtering_ostream zos;
        zos.push(boost::iostreams::zlib_compressor(params));
        zos.push(os);

        freeorion_xml_oarchive oa(zos);
        oa << BOOST_SERIALIZATION_NVP(data);
    }
    return Message{Message::MessageType::SOME_MSG, std::move(os).str()};
}

Would it be preferable for the user option to be the compression level, rather than just a true/false toggle?

[o01eg]

Hard to say, in most cases big messages are sent from server to client.

[mlam19]

Here's the code for the user options and client-server compression usage requests. I have no idea how to perform Git operations between GitHub forks, so patch files it is.

master...mlam19:msg_data_cpr_v

0001-Stream-based-compression-decompression.patch.txt
0002-Add-compression-usage-option.patch.txt
0003-Add-client-server-compression-usage-requests.patch.txt

[o01eg]

I think we don't need compression for every message because many of them are quite small.

[mlam19]

That's fair. I think the messages that tend to get very large are:

• DISPATCH_COMBAT_LOGS
• GAME_START
• TURN_PARTIAL_UPDATE
• TURN_UPDATE

[o01eg]

CHAT_HISTORY tend to be large too.

[mlam19]

0001-Message-compression.patch.txt

• Compression is limited to only larger message types.
• Compression is only used for non-localhost clients
• There's no user selectability
• The filtering_ostream::reset() were removed; those seem to break things
• Removed the string comparison for detecting the use of binary serialization; not sure if that would work anymore

EDIT: Updated patch with changes to more message creation calls. filtering_ostream::reset() readded but seems to cause server crash in Message::ChatHistoryMessage().

[o01eg]

The filtering_ostream::reset() were removed; those seem to break things

@mlam19 Did you catch issue with it?

[mlam19]

Since the last patch, I've found and set some more use_compression flags in various message creation calls. So now the server crash occurs when filtering_ostream::reset() is used is in Message::ChatHistoryMessage() when a remote client joins a game. I added some debug messages and exception handling.

Message ChatHistoryMessage(const std::vector<std::reference_wrapper<const ChatHistoryEntity>>& chat_history,
                           bool use_compression) {
    std::ostringstream os;
    {
        try {
            boost::iostreams::zlib_params params;
            params.level = boost::iostreams::zlib::no_compression;
            if (use_compression)
                params.level = boost::iostreams::zlib::default_compression;
            boost::iostreams::filtering_ostream zos;
            zos.push(boost::iostreams::zlib_compressor(params));
            zos.push(os);
            freeorion_xml_oarchive oa(zos);
            std::size_t size = chat_history.size();
            oa << BOOST_SERIALIZATION_NVP(size);
            for (const auto &elem : chat_history) {
                oa << boost::serialization::make_nvp(BOOST_PP_STRINGIZE(elem), elem.get());
            }
            if (!zos.strict_sync()) {
                DebugLogger() << "D1";
                zos.reset();
            }
            DebugLogger() << "D2"; // Last debug message displayed
        } catch (const std::exception &err) {
            DebugLogger() << "D3";
            ErrorLogger() << "ChatHistoryMessage:\n" << "Error: " << err.what();
            throw err;
        }
        DebugLogger() << "D4";
    }
    return Message{Message::MessageType::CHAT_HISTORY, std::move(os).str()};
}

The last messages in the server log are the "D1" and "D2" debug messages. Nothing appears in the terminal.

[geoffthemedio]

the server crash occurs when filtering_ostream::reset() is used
[...]
The last messages in the server log are the "D1" and "D2" debug messages. Nothing appears in the terminal.

By "when" do you mean "if"? Because if the log message for "D2" appears in the log, then the .reset() apparently already ran.

[mlam19]

Yes, the "D1" and "D2" indicates that the .reset() has run.

If the .reset() is commented out, then the "D1", "D2" and "D4" messages are logged. After that, the game can be started, but then the remote client cannot progress beyond turn 1 (turn button does nothing.) On the other hand, the server doesn't crash.

[mlam19]

Found my problem. xml_oarchive only finishes writing when it is destroyed. In Message::ChatHistoryMessage(), the filtering_ostream was being rendered unusable by reset() before the xml_oarchive was destroyed at the end of the nested block; this caused a segmentation fault during the destruction of the xml_oarchive.

The xml_oarchive is now encapsulated in its own nested block to ensure it is destroyed before the reset().

Here's the updated patch:

0001-Message-compression.patch.txt

[mlam19]

I've made a new pull request (#3904) with the patch.