From aea95b5be046645cc93106183a258ee52623c6f6 Mon Sep 17 00:00:00 2001 From: "Dirk-Jan C. Binnema" Date: Mon, 31 Jul 2023 23:53:29 +0300 Subject: [PATCH] mu-server: use strings, not sexps object (optimization) When passing messages to mu, often we got a (parsed from string) message-sexp from the message document; then appended some more properties ("build_message_sexp"). Instead, we can do it in terms of the strings; this is _a little_ inelegant, but also much faster; compare: (base) [mu4e] Found 500 matching messages; 0 hidden; search: 1298.0 ms (2.60 ms/msg); render: 642.1 ms (1.28 ms/msg) (with temp-file optimization (earlier commit) [mu4e] Found 500 matching messages; 0 hidden; search: 1152.7 ms (2.31 ms/msg); render: 270.1 ms (0.54 ms/msg) (with temp file optimize _and_ the string opt (this commit) [mu4e] Found 500 matching messages; 0 hidden; search: 266.0 ms (0.53 ms/msg); render: 199.7 ms (0.40 ms/msg) --- lib/message/mu-document.hh | 10 +- lib/mu-server.cc | 213 ++++++++++++++++++++----------------- lib/mu-server.hh | 9 +- mu/mu-cmd-server.cc | 23 ++-- 4 files changed, 137 insertions(+), 118 deletions(-) diff --git a/lib/message/mu-document.hh b/lib/message/mu-document.hh index 9aef65db..21de4e20 100644 --- a/lib/message/mu-document.hh +++ b/lib/message/mu-document.hh @@ -144,13 +144,19 @@ public: void remove(Field::Id field_id); /** - * Get the cached s-expression useful for changing - * it (call update_sexp_cache() when done) + * Get the cached s-expression * * @return the cached s-expression */ const Sexp& sexp() const { return cached_sexp(); } + /** + * Get the message s-expression as a string + * + * @return message s-expression string + */ + std::string sexp_str() const { return xdoc_.get_data(); } + /** * Generically adds an optional value, if set, to the document * diff --git a/lib/mu-server.cc b/lib/mu-server.cc index f6203040..575412d1 100644 --- a/lib/mu-server.cc +++ b/lib/mu-server.cc @@ -84,10 +84,12 @@ struct Server::Private { // // output - // - void output_sexp(const Sexp& sexp, Server::OutputFlags flags = {}) const { + void output(const std::string& str, Server::OutputFlags flags = {}) const { if (output_) - output_(sexp, flags); + output_(str, flags); + } + void output_sexp(const Sexp& sexp, Server::OutputFlags flags = {}) const { + output(sexp.to_string(), flags); } size_t output_results(const QueryResults& qres, size_t batch_size) const; @@ -112,11 +114,6 @@ struct Server::Private { void view_handler(const Command& cmd); private: - // helpers - Sexp build_message_sexp(const Message& msg, - Store::Id docid, - const Option qm) const; - void move_docid(Store::Id docid, Option flagstr, bool new_name, bool no_view); @@ -140,54 +137,66 @@ private: std::string tmp_dir_; }; -static Sexp -build_metadata(const QueryMatch& qmatch) +static void +append_metadata(std::string& str, const QueryMatch& qmatch) { const auto td{::atoi(qmatch.thread_date.c_str())}; - auto mdata = Sexp().put_props(":path", qmatch.thread_path, - ":level", qmatch.thread_level, - ":date", qmatch.thread_date, - ":data-tstamp", Sexp().add(static_cast(td >> 16), - static_cast(td & 0xffff), - 0)); - if (qmatch.has_flag(QueryMatch::Flags::Root)) - mdata.put_props(":root", Sexp::t_sym); - if (qmatch.has_flag(QueryMatch::Flags::Related)) - mdata.put_props(":related", Sexp::t_sym); - if (qmatch.has_flag(QueryMatch::Flags::First)) - mdata.put_props(":first-child", Sexp::t_sym); - if (qmatch.has_flag(QueryMatch::Flags::Last)) - mdata.put_props(":last-child", Sexp::t_sym); - if (qmatch.has_flag(QueryMatch::Flags::Orphan)) - mdata.put_props(":orphan", Sexp::t_sym); - if (qmatch.has_flag(QueryMatch::Flags::Duplicate)) - mdata.put_props(":duplicate", Sexp::t_sym); - if (qmatch.has_flag(QueryMatch::Flags::HasChild)) - mdata.put_props(":has-child", Sexp::t_sym); - if (qmatch.has_flag(QueryMatch::Flags::ThreadSubject)) - mdata.put_props(":thread-subject", Sexp::t_sym); - return mdata; + str += mu_format(" :meta (:path \"{}\" :level {} :date \"{}\" " + ":data-tstamp ({} {} 0)", + qmatch.thread_path, + qmatch.thread_level, + qmatch.thread_date, + static_cast(td >> 16), + static_cast(td & 0xffff)); + + if (qmatch.has_flag(QueryMatch::Flags::Root)) + str += " :root t"; + if (qmatch.has_flag(QueryMatch::Flags::Related)) + str += " :related t"; + if (qmatch.has_flag(QueryMatch::Flags::First)) + str += " :first-child t"; + if (qmatch.has_flag(QueryMatch::Flags::Last)) + str += " :last-child t"; + if (qmatch.has_flag(QueryMatch::Flags::Orphan)) + str += " :orphan t"; + if (qmatch.has_flag(QueryMatch::Flags::Duplicate)) + str += " :duplicate t"; + if (qmatch.has_flag(QueryMatch::Flags::HasChild)) + str += " :has-child t"; + if (qmatch.has_flag(QueryMatch::Flags::ThreadSubject)) + str += " :thread-subject t"; + + str += ')'; } /* * A message here consists of a message s-expression with optionally a :docid * and/or :meta expression added. + * + * Note, for speed reasons, we build is as a _string_, without any further + * parsing. */ -Sexp -Server::Private::build_message_sexp(const Message& msg, - Store::Id docid, - const Option qm) const +static std::string +msg_sexp_str(const Message& msg, Store::Id docid, const Option qm) { - Sexp sexp{msg.sexp()}; // copy - if (docid != 0) - sexp.put_props(":docid", docid); - if (qm) - sexp.put_props(":meta", build_metadata(*qm)); + auto sexpstr{msg.document().sexp_str()}; + sexpstr.reserve(sexpstr.size () + (docid == 0 ? 0 : 16) + (qm ? 64 : 0)); - return sexp; + // remove the closing ( ... ) + sexpstr.erase(sexpstr.end() - 1); + + if (docid != 0) + sexpstr += " :docid " + to_string(docid); + if (qm) + append_metadata(sexpstr, *qm); + + sexpstr += ')'; // ... end close it again. + + return sexpstr; } + CommandHandler::CommandInfoMap Server::Private::make_command_map() { @@ -413,8 +422,8 @@ Server::Private::add_handler(const Command& cmd) throw Error(Error::Code::Store, "failed to get message at {} (docid={})", *path, docid); - output_sexp(Sexp().put_props(":update", - build_message_sexp(msg_res.value(), docid, {}))); + output(mu_format("(:update {})", + msg_sexp_str(msg_res.value(), docid, {}))); } /* 'compose' produces the un-changed *original* message sexp (ie., the message @@ -423,8 +432,8 @@ Server::Private::add_handler(const Command& cmd) * edit/resend), and 'docid' for the message to reply to. Note, type:new does * not have an original message, and therefore does not need a docid * - * In returns a (:compose [:original ] [:include] ) - * message (detals: see code below) + * In returns a (:compose [:original ] [:include] ) message + * (details: see code below) * * Note ':include' t or nil determines whether to include attachments */ @@ -476,8 +485,8 @@ Server::Private::compose_handler(const Command& cmd) if (!msg) throw Error{Error::Code::Store, "failed to get message {}", docid}; - comp_lst.put_props(":original", build_message_sexp(msg.value(), docid, {})); - + auto msg_sexp = unwrap(Sexp::parse(msg_sexp_str(msg.value(), docid, {}))); + comp_lst.put_props(":original", msg_sexp); if (ctype == "forward") { // when forwarding, attach any attachment in the orig size_t index{}; @@ -510,8 +519,8 @@ Server::Private::make_temp_file_stream() const if (!output.good()) throw Mu::Error{Error::Code::File, "failed to create temp-file"}; - return make_pair(std::move(output), - std::move(tmp_eld)); + return std::make_pair(std::move(output), + std::move(tmp_eld)); } @@ -531,37 +540,48 @@ Server::Private::contacts_handler(const Command& cmd) mu_debug("find {} contacts last seen >= {} (tstamp: {})", personal ? "personal" : "any", time_to_string("%c", after), tstamp); - auto n{0}; - Sexp contacts; - store().contacts_cache().for_each([&](const Contact& ci) { - - /* since the last time we got some contacts */ + auto match_contact = [&](const Contact& ci)->bool { if (tstamp > ci.tstamp) - return true; - /* (maybe) only include 'personal' contacts */ - if (personal && !ci.personal) - return true; - /* only include newer-than-x contacts */ - if (after > ci.message_date) - return true; - n++; - contacts.add(ci.display_name()); - return maxnum == 0 || n < maxnum; - }); + return false; /* already seen? */ + else if (personal && !ci.personal) + return false; /* not personal? */ + else if (after > ci.message_date) + return true; /* too old? */ + else + return false; + }; - /* dump the contacts cache as a giant sexp */ - mu_debug("sending {} of {} contact(s)", n, store().contacts_cache().size()); + auto n{0}; if (options_.allow_temp_file) { - auto&& [output, tmp_eld] = make_temp_file_stream(); - output << contacts; - output_sexp(Sexp{":tstamp"_sym, mu_format("{}", g_get_monotonic_time()), - ":contacts-temp-file"_sym, tmp_eld}); + // fast way: put all the contacts in a temp-file + auto&& [tmp_file, tmp_file_name] = make_temp_file_stream(); + tmp_file << '('; + store().contacts_cache().for_each([&](const Contact& ci) { + if (!match_contact(ci)) + return true; // continue + tmp_file << quote(ci.display_name()) << "\n"; + ++n; + return maxnum == 0 || n < maxnum; + }); + tmp_file << ')'; + output_sexp(Sexp{":tstamp"_sym, mu_format("\"{}\"", g_get_monotonic_time()), + ":contacts-temp-file"_sym, tmp_file_name}); } else { + Sexp contacts; + store().contacts_cache().for_each([&](const Contact& ci) { + if (!match_contact(ci)) + return true; // continue; + contacts.add(ci.display_name()); + ++n; + return maxnum == 0 || n < maxnum; + }); Sexp seq; - seq.put_props(":contacts", contacts, - ":tstamp", mu_format("{}", g_get_monotonic_time())); - output_sexp(seq, Server::OutputFlags::SplitList); + seq.put_props(":contacts", std::move(contacts), + ":tstamp", mu_format("\"{}\"", g_get_monotonic_time())); + output_sexp(seq); } + + mu_debug("sent {} of {} contact(s)", n, store().contacts_cache().size()); } /* @@ -601,7 +621,7 @@ determine_docids(const Store& store, const Command& cmd) size_t Server::Private::output_results(const QueryResults& qres, size_t batch_size) const { - size_t n{}; + size_t n{}; Sexp headers; const auto output_batch = [&](Sexp&& hdrs) { @@ -617,7 +637,7 @@ Server::Private::output_results(const QueryResults& qres, size_t batch_size) con ++n; // construct sexp for a single header. auto qm{mi.query_match()}; - auto msgsexp{build_message_sexp(*msg, mi.doc_id(), qm)}; + auto msgsexp{unwrap(Sexp::parse(msg_sexp_str(*msg, mi.doc_id(), qm)))}; headers.add(std::move(msgsexp)); // we output up-to-batch-size lists of messages. It's much // faster (on the emacs side) to handle such batches than single @@ -641,8 +661,8 @@ Server::Private::output_results_temp_file(const QueryResults& qres, size_t batch // create an output stream with a file name size_t n{}; - auto&& [output, tmp_eld] = make_temp_file_stream(); - output << '('; + auto&& [tmp_file, tmp_file_name] = make_temp_file_stream(); + tmp_file << '('; for(auto&& mi: qres) { auto msg{mi.message()}; @@ -650,20 +670,21 @@ Server::Private::output_results_temp_file(const QueryResults& qres, size_t batch continue; auto qm{mi.query_match()}; // construct sexp for a single header. - output << build_message_sexp(*msg, mi.doc_id(), qm); + tmp_file << msg_sexp_str(*msg, mi.doc_id(), qm) << '\n'; ++n; if (n % batch_size == 0) { - output << ')'; - output_sexp(Sexp{":headers-temp-file"_sym, tmp_eld}); + tmp_file << ')'; + batch_size = 1000; + output_sexp(Sexp{":headers-temp-file"_sym, tmp_file_name}); auto new_stream{make_temp_file_stream()}; - output = std::move(new_stream.first); - tmp_eld = std::move(new_stream.second); - output << '('; + tmp_file = std::move(new_stream.first); + tmp_file_name = std::move(new_stream.second); + tmp_file << '('; } } - output << ')'; - output_sexp(Sexp{":headers-temp-file"_sym, tmp_eld}); + tmp_file << ')'; + output_sexp(Sexp{":headers-temp-file"_sym, tmp_file_name}); return n; } @@ -855,18 +876,18 @@ Server::Private::perform_move(Store::Id docid, throw idmsgvec.error(); for (auto&&[id, msg]: *idmsgvec) { - Sexp sexp{":update"_sym, build_message_sexp(idmsgvec->at(0).second, id, {})}; + auto sexpstr = "(:update " + msg_sexp_str(idmsgvec->at(0).second, id, {}); /* note, the :move t thing is a hint to the frontend that it * could remove the particular header */ if (different_mdir) - sexp.put_props(":move", Sexp::t_sym); + sexpstr += " :move t"; if (!no_view && id == docid) - sexp.put_props(":maybe-view", Sexp::t_sym); - output_sexp(std::move(sexp)); + sexpstr += " :maybe-view t"; + sexpstr += ')'; + output(std::move(sexpstr)); } } - static Flags calculate_message_flags(const Message& msg, Option flagopt) { @@ -1042,8 +1063,8 @@ Server::Private::view_mark_as_read(Store::Id docid, Message&& msg, bool rename) throw move_res.error(); for (auto&& [id, msg]: move_res.value()) - output_sexp(Sexp{id == docid ? ":view"_sym : ":update"_sym, - build_message_sexp(msg, id, {})}); + output(mu_format("({} {})", id == docid ? ":view" : ":update", + msg_sexp_str(msg, id, {}))); } void @@ -1067,7 +1088,7 @@ Server::Private::view_handler(const Command& cmd) /* if the message should not be marked-as-read, we're done. */ if (!mark_as_read) - output_sexp(Sexp().put_props(":view", build_message_sexp(msg, docid, {}))); + output(mu_format("(:view {})", msg_sexp_str(msg, docid, {}))); else view_mark_as_read(docid, std::move(msg), rename); /* otherwise, mark message and and possible dups as read */ diff --git a/lib/mu-server.hh b/lib/mu-server.hh index 4946443a..28d2da3f 100644 --- a/lib/mu-server.hh +++ b/lib/mu-server.hh @@ -36,19 +36,16 @@ class Server { public: enum struct OutputFlags { None = 0, - SplitList = 1 << 0, - /**< insert newlines between list items */ - Flush = 1 << 1, - /**< flush output buffer after */ + Flush = 1 << 0, /**< flush output buffer after */ }; /** * Prototype for output function * - * @param sexp an s-expression + * @param str a string * @param flags flags that influence the behavior */ - using Output = std::function; + using Output = std::function; struct Options { bool allow_temp_file; /**< temp file optimization allowed? */ diff --git a/mu/mu-cmd-server.cc b/mu/mu-cmd-server.cc index 6a27b789..b39fa1dc 100644 --- a/mu/mu-cmd-server.cc +++ b/mu/mu-cmd-server.cc @@ -80,32 +80,27 @@ cookie(size_t n) ::printf(COOKIE_PRE "%x" COOKIE_POST, num); } -static void -output_sexp_stdout(const Sexp& sexp, Server::OutputFlags flags) -{ - /* if requested, insert \n between list elements; note: - * is _not_ inherited by children */ - Sexp::Format fopts{}; - if (any_of(flags & Server::OutputFlags::SplitList)) - fopts |= Sexp::Format::SplitList; - const auto str{sexp.to_string(fopts)}; + +static void +output_stdout(const std::string& str, Server::OutputFlags flags) +{ cookie(str.size() + 1); if (G_UNLIKELY(::puts(str.c_str()) < 0)) { mu_critical("failed to write output '{}'", str); ::raise(SIGTERM); /* terminate ourselves */ } - if (any_of(flags & Server::OutputFlags::Flush)) std::fflush(stdout); } + static void report_error(const Mu::Error& err) noexcept { - output_sexp_stdout(Sexp(":error"_sym, Error::error_number(err.code()), - ":message"_sym, err.what()), - Server::OutputFlags::Flush); + output_stdout(Sexp(":error"_sym, Error::error_number(err.code()), + ":message"_sym, err.what()).to_string(), + Server::OutputFlags::Flush); } @@ -120,7 +115,7 @@ Mu::mu_cmd_server(const Mu::Options& opts) try { Server::Options sopts{}; sopts.allow_temp_file = opts.server.allow_temp_file; - Server server{*store, sopts, output_sexp_stdout}; + Server server{*store, sopts, output_stdout}; mu_message("created server with store @ {}; maildir @ {}; debug-mode {};" "readline: {}", store->path(), store->root_maildir(),