From 8b3b8a6e3c9dbf3b10460a396983c608db21f4dc Mon Sep 17 00:00:00 2001 From: Timothy Stack Date: Thu, 7 Jul 2022 22:13:18 -0700 Subject: [PATCH] [perf] fix some text file loading issues --- docs/schemas/format-v1.schema.json | 6 ++++++ src/base/date_time_scanner.cc | 9 +++++++++ src/base/date_time_scanner.hh | 8 +------- src/field_overlay_source.cc | 3 ++- src/formats/vmk_log.json | 1 + src/line_buffer.cc | 7 ------- src/listview_curses.cc | 16 ++++++++++------ src/listview_curses.hh | 3 +++ src/lnav.cc | 16 ++++++++++------ src/lnav_commands.cc | 10 +++++++--- src/log_format.hh | 6 ++++-- src/log_format_impls.cc | 10 ++++++---- src/log_format_loader.cc | 7 +++++++ src/logfile.cc | 29 +++++++++++++++++++++++------ src/logfile.cfg.hh | 2 +- src/logfile.hh | 2 ++ src/spectro_source.cc | 22 ++++++++++++++++++---- src/textfile_sub_source.cc | 6 +++--- src/yajlpp/yajlpp.hh | 10 ++++++++++ src/yajlpp/yajlpp_def.hh | 22 ++++++++++++++++++++++ test/test_logfile.sh | 1 - 21 files changed, 145 insertions(+), 51 deletions(-) diff --git a/docs/schemas/format-v1.schema.json b/docs/schemas/format-v1.schema.json index 687d7236..2b796c74 100644 --- a/docs/schemas/format-v1.schema.json +++ b/docs/schemas/format-v1.schema.json @@ -503,6 +503,12 @@ "json", "csv" ] + }, + "max-unrecognized-lines": { + "title": "//max-unrecognized-lines", + "description": "The maximum number of lines in a file to use when detecting the format", + "type": "integer", + "minimum": 1 } }, "additionalProperties": false diff --git a/src/base/date_time_scanner.cc b/src/base/date_time_scanner.cc index 346f5bba..bc8f75e2 100644 --- a/src/base/date_time_scanner.cc +++ b/src/base/date_time_scanner.cc @@ -269,6 +269,15 @@ date_time_scanner::scan(const char* time_dest, return retval; } +void +date_time_scanner::set_base_time(time_t base_time, const tm& local_tm) +{ + this->dts_base_time = base_time; + this->dts_base_tm.et_tm = local_tm; + this->dts_last_tm = exttm{}; + this->dts_last_tv = timeval{}; +} + void date_time_scanner::to_localtime(time_t t, exttm& tm_out) { diff --git a/src/base/date_time_scanner.hh b/src/base/date_time_scanner.hh index c415c6d7..90eaffa2 100644 --- a/src/base/date_time_scanner.hh +++ b/src/base/date_time_scanner.hh @@ -68,13 +68,7 @@ struct date_time_scanner { this->dts_fmt_len = -1; } - void set_base_time(time_t base_time) - { - this->dts_base_time = base_time; - localtime_r(&base_time, &this->dts_base_tm.et_tm); - this->dts_last_tm = exttm{}; - this->dts_last_tv = timeval{}; - } + void set_base_time(time_t base_time, const tm& local_tm); /** * Convert a timestamp to local time. diff --git a/src/field_overlay_source.cc b/src/field_overlay_source.cc index dd052bf6..f7f21f8b 100644 --- a/src/field_overlay_source.cc +++ b/src/field_overlay_source.cc @@ -152,7 +152,8 @@ field_overlay_source::build_field_lines(const listview_curses& lv) date_time_scanner dts; struct exttm tm; - dts.set_base_time(format->lf_date_time.dts_base_time); + dts.set_base_time(format->lf_date_time.dts_base_time, + format->lf_date_time.dts_base_tm.et_tm); if (format->lf_date_time.scan(time_src, time_range.length(), format->get_timestamp_formats(), diff --git a/src/formats/vmk_log.json b/src/formats/vmk_log.json index 63106331..b59a2553 100644 --- a/src/formats/vmk_log.json +++ b/src/formats/vmk_log.json @@ -14,6 +14,7 @@ "error": "ALERT", "warning": "WARNING" }, + "max-unrecognized-lines": 15000, "value": { "cpu": { "kind": "integer", diff --git a/src/line_buffer.cc b/src/line_buffer.cc index dcadf2b0..ca247ca2 100644 --- a/src/line_buffer.cc +++ b/src/line_buffer.cc @@ -398,7 +398,6 @@ line_buffer::set_fd(auto_fd& fd) this->lb_seekable = true; } } - log_debug("newoff %d", newoff); this->lb_file_offset = newoff; this->lb_buffer.clear(); this->lb_fd = std::move(fd); @@ -839,12 +838,6 @@ line_buffer::fill_range(file_off_t start, ssize_t max_length) #endif else if (this->lb_seekable) { -#if 1 - log_debug("doing pread file_off=%d read_off=%d count=%d", - this->lb_file_offset, - this->lb_file_offset + this->lb_buffer.size(), - this->lb_buffer.available()); -#endif rc = pread(this->lb_fd, this->lb_buffer.end(), this->lb_buffer.available(), diff --git a/src/listview_curses.cc b/src/listview_curses.cc index ce3cf0c8..28942444 100644 --- a/src/listview_curses.cc +++ b/src/listview_curses.cc @@ -196,7 +196,6 @@ listview_curses::do_update() attr_line_t overlay_line; struct line_range lr; unsigned long width, wrap_width; - size_t row_count; int y = this->lv_y, bottom; attr_t role_attrs = vc.attrs_for_role(this->vc_default_role); @@ -211,7 +210,7 @@ listview_curses::do_update() : this->lv_show_scrollbar ? 1 : 0); - row_count = this->get_inner_height(); + size_t row_count = this->get_inner_height(); row = this->lv_top; bottom = y + height; std::vector rows( @@ -484,13 +483,17 @@ listview_curses::set_top(vis_line_t top, bool suppress_flash) if (this->lv_selectable) { if (this->lv_selection < 0_vl) { } else if (this->lv_selection < top) { - this->lv_selection = top; + this->set_selection(top); } else { auto bot = this->get_bottom(); + unsigned long width; + vis_line_t height; - if (bot != -1_vl) { + this->get_dimensions(height, width); + + if (bot != -1_vl && (bot - top) >= (height - 1)) { if (this->lv_selection > (bot - this->lv_tail_space)) { - this->lv_selection = bot - this->lv_tail_space; + this->set_selection(bot - this->lv_tail_space); } } } @@ -567,7 +570,8 @@ listview_curses::scroll_selection_into_view() if (this->lv_selection < 0_vl) { this->set_top(0_vl); } else if (this->lv_selection - >= (this->lv_top + height - this->lv_tail_space)) { + >= (this->lv_top + height - this->lv_tail_space - 1_vl)) + { this->set_top(this->lv_selection - height + 1_vl + this->lv_tail_space, true); } else if (this->lv_selection < this->lv_top) { diff --git a/src/listview_curses.hh b/src/listview_curses.hh index 7035b3a3..b69088ac 100644 --- a/src/listview_curses.hh +++ b/src/listview_curses.hh @@ -458,6 +458,9 @@ public: if (this->lv_height < 0) { height_out = vis_line_t(height) + this->lv_height - vis_line_t(this->lv_y); + if (height_out < 0_vl) { + height_out = 0_vl; + } } else { height_out = this->lv_height; } diff --git a/src/lnav.cc b/src/lnav.cc index 88147725..97c8b2df 100644 --- a/src/lnav.cc +++ b/src/lnav.cc @@ -2186,7 +2186,6 @@ main(int argc, char* argv[]) "missing format files to install") .with_reason(install_reason) .with_help(install_help)); - usage(); return EXIT_FAILURE; } @@ -2771,19 +2770,24 @@ SELECT tbl_name FROM sqlite_master WHERE sql LIKE 'CREATE VIRTUAL TABLE%' } } - if (lnav_data.ld_active_files.fc_file_names.empty() + if (retval == EXIT_SUCCESS + && lnav_data.ld_active_files.fc_file_names.empty() && lnav_data.ld_commands.empty() && lnav_data.ld_pt_search.empty() && !(lnav_data.ld_show_help_view || mode_flags.mf_no_default)) { lnav::console::print( stderr, - lnav::console::user_message::error("no log files given/found")); + lnav::console::user_message::error("nothing to do") + .with_reason("no files given or default files found") + .with_help( + attr_line_t("use the ") + .append_quoted(lnav::roles::keyword("-N")) + .append( + " option to open lnav without loading any files"))); retval = EXIT_FAILURE; } - if (retval != EXIT_SUCCESS) { - usage(); - } else { + if (retval == EXIT_SUCCESS) { isc::supervisor root_superv(injector::get()); try { diff --git a/src/lnav_commands.cc b/src/lnav_commands.cc index 8b3d0bfc..2e3adf17 100644 --- a/src/lnav_commands.cc +++ b/src/lnav_commands.cc @@ -229,17 +229,19 @@ com_adjust_log_time(exec_context& ec, date_time_scanner dts; vis_line_t top_line; struct exttm tm; + struct tm base_tm; std::shared_ptr lf; top_line = lnav_data.ld_views[LNV_LOG].get_selection(); top_content = lss.at(top_line); lf = lss.find(top_content); - logline& ll = (*lf)[top_content]; + auto& ll = (*lf)[top_content]; top_time = ll.get_timeval(); + localtime_r(&top_time.tv_sec, &base_tm); - dts.set_base_time(top_time.tv_sec); + dts.set_base_time(top_time.tv_sec, base_tm); args[1] = remaining_args(cmdline, args); auto parse_res = relative_time::from_str(args[1]); @@ -3279,11 +3281,13 @@ com_pt_time(exec_context& ec, date_time_scanner dts; struct exttm tm; time_t now; + struct tm base_tm; auto parse_res = relative_time::from_str(all_args); time(&now); + localtime_r(&now, &base_tm); dts.dts_keep_base_tz = true; - dts.set_base_time(now); + dts.set_base_time(now, base_tm); if (parse_res.isOk()) { tm.et_tm = *gmtime(&now); tm = parse_res.unwrap().adjust(tm); diff --git a/src/log_format.hh b/src/log_format.hh index c6cb5a31..9201913c 100644 --- a/src/log_format.hh +++ b/src/log_format.hh @@ -489,14 +489,16 @@ public: bool lf_is_self_describing{false}; bool lf_time_ordered{true}; bool lf_specialized{false}; + nonstd::optional lf_max_unrecognized_lines; protected: static std::vector> lf_root_formats; struct pcre_format { - pcre_format(const char* regex) : name(regex), pcre(regex) + explicit pcre_format(const char* regex) + : name(regex), pcre(regex), + pf_timestamp_index(this->pcre.name_index("timestamp")) { - this->pf_timestamp_index = this->pcre.name_index("timestamp"); } pcre_format() : name(nullptr), pcre("") {} diff --git a/src/log_format_impls.cc b/src/log_format_impls.cc index 5c9b7d08..ad9529b3 100644 --- a/src/log_format_impls.cc +++ b/src/log_format_impls.cc @@ -1070,8 +1070,10 @@ public: nullptr, &tm, tv)) { - this->lf_date_time.set_base_time(tv.tv_sec); - this->wlf_time_scanner.set_base_time(tv.tv_sec); + this->lf_date_time.set_base_time(tv.tv_sec, + tm.et_tm); + this->wlf_time_scanner.set_base_time(tv.tv_sec, + tm.et_tm); } } } @@ -1210,8 +1212,8 @@ public: &tm, tv)) { - this->lf_date_time.set_base_time(tv.tv_sec); - this->wlf_time_scanner.set_base_time(tv.tv_sec); + this->lf_date_time.set_base_time(tv.tv_sec, tm.et_tm); + this->wlf_time_scanner.set_base_time(tv.tv_sec, tm.et_tm); } } else if (directive == "#Fields:" && this->wlf_field_defs.empty()) { diff --git a/src/log_format_loader.cc b/src/log_format_loader.cc index 4f6b62df..b46f1da6 100644 --- a/src/log_format_loader.cc +++ b/src/log_format_loader.cc @@ -869,6 +869,13 @@ struct json_path_container format_handlers = { .with_description("The type of file that contains the log messages") .with_enum_values(TYPE_ENUM) .for_field(&external_log_format::elf_type), + + yajlpp::property_handler("max-unrecognized-lines") + .with_synopsis("") + .with_description("The maximum number of lines in a file to use when " + "detecting the format") + .with_min_value(1) + .for_field(&log_format::lf_max_unrecognized_lines), }; static int diff --git a/src/logfile.cc b/src/logfile.cc index 1352140b..8117a1cc 100644 --- a/src/logfile.cc +++ b/src/logfile.cc @@ -172,7 +172,17 @@ logfile::set_format_base_time(log_format* lf) if (file_time == 0) { file_time = this->lf_stat.st_mtime; } - lf->lf_date_time.set_base_time(file_time); + + if (!this->lf_cached_base_time + || this->lf_cached_base_time.value() != file_time) + { + struct tm new_base_tm; + this->lf_cached_base_time = file_time; + localtime_r(&file_time, &new_base_tm); + this->lf_cached_base_tm = new_base_tm; + } + lf->lf_date_time.set_base_time(this->lf_cached_base_time.value(), + this->lf_cached_base_tm.value()); } bool @@ -180,6 +190,10 @@ logfile::process_prefix(shared_buffer_ref& sbr, const line_info& li, scan_batch_context& sbc) { + static auto max_unrecognized_lines + = injector::get() + .lc_max_unrecognized_lines; + log_format::scan_result_t found = log_format::SCAN_NO_MATCH; size_t prescan_size = this->lf_index.size(); time_t prescan_time = 0; @@ -191,11 +205,7 @@ logfile::process_prefix(shared_buffer_ref& sbr, } /* We've locked onto a format, just use that scanner. */ found = this->lf_format->scan(*this, this->lf_index, li, sbr, sbc); - } else if (this->lf_options.loo_detect_format - && this->lf_index.size() - < injector::get() - .lc_max_unrecognized_lines) - { + } else if (this->lf_options.loo_detect_format) { const auto& root_formats = log_format::get_root_formats(); /* @@ -206,6 +216,13 @@ logfile::process_prefix(shared_buffer_ref& sbr, iter != root_formats.end() && (found != log_format::SCAN_MATCH); ++iter) { + if (this->lf_index.size() + >= (*iter)->lf_max_unrecognized_lines.value_or( + max_unrecognized_lines)) + { + continue; + } + if (!(*iter)->match_name(this->lf_filename)) { if (li.li_file_range.fr_offset == 0) { log_debug("(%s) does not match file name: %s", diff --git a/src/logfile.cfg.hh b/src/logfile.cfg.hh index f249d113..8b0f68b0 100644 --- a/src/logfile.cfg.hh +++ b/src/logfile.cfg.hh @@ -36,7 +36,7 @@ namespace lnav { namespace logfile { struct config { - uint64_t lc_max_unrecognized_lines{15000}; + uint64_t lc_max_unrecognized_lines{1000}; }; } // namespace logfile diff --git a/src/logfile.hh b/src/logfile.hh index ba97eba3..3cd9f986 100644 --- a/src/logfile.hh +++ b/src/logfile.hh @@ -420,6 +420,8 @@ private: safe_opid_map lf_opids; size_t lf_watch_count{0}; ArenaAlloc::Alloc lf_allocator{64 * 1024}; + nonstd::optional lf_cached_base_time; + nonstd::optional lf_cached_base_tm; nonstd::optional> lf_next_line_cache; }; diff --git a/src/spectro_source.cc b/src/spectro_source.cc index 0183492c..f82e02c7 100644 --- a/src/spectro_source.cc +++ b/src/spectro_source.cc @@ -225,6 +225,10 @@ spectrogram_source::list_value_for_overlay(const listview_curses& lv, auto mark_offset = this->ss_cursor_column.value(); auto mark_is_before = true; + if (desc.length() + 8 > width) { + desc.clear(); + } + if (this->ss_cursor_column.value() + desc.length() + 1 > width) { mark_offset -= desc.length(); mark_is_before = false; @@ -287,16 +291,26 @@ spectrogram_source::list_value_for_overlay(const listview_curses& lv, st.st_yellow_threshold - 1, role_t::VCR_HIGH_THRESHOLD, st.st_yellow_threshold); - line.append(width / 2 - strlen(buf) / 3 - line.length(), ' '); + auto buflen = strlen(buf); + if (line.length() + buflen + 20 < width) { + line.append(width / 2 - buflen / 3 - line.length(), ' '); + } else { + line.append(" "); + } line.append(buf); scrub_ansi_string(line, value_out.get_attrs()); snprintf(buf, sizeof(buf), "Max: %'.10lg", sb.sb_max_value_out); - line.append(width - strlen(buf) - line.length() - 2, ' '); + buflen = strlen(buf); + if (line.length() + buflen + 4 < width) { + line.append(width - buflen - line.length() - 2, ' '); + } else { + line.append(" "); + } line.append(buf); - value_out.with_attr(string_attr(line_range(0, -1), - VC_STYLE.value(A_UNDERLINE))); + value_out.with_attr( + string_attr(line_range(0, -1), VC_STYLE.value(A_UNDERLINE))); return true; } diff --git a/src/textfile_sub_source.cc b/src/textfile_sub_source.cc index c39572cd..9f5d8c2b 100644 --- a/src/textfile_sub_source.cc +++ b/src/textfile_sub_source.cc @@ -279,9 +279,6 @@ textfile_sub_source::text_crumbs_for_line( } auto lf = this->current_file(); - if (lf->size() == 0) { - return; - } crumbs.emplace_back( lf->get_unique_path(), attr_line_t().append(lnav::roles::identifier(lf->get_unique_path())), @@ -308,6 +305,9 @@ textfile_sub_source::text_crumbs_for_line( this->to_front(lf_opt.value()); this->tss_view->reload_data(); }); + if (lf->size() == 0) { + return; + } auto rend_iter = this->tss_rendered_files.find(lf->get_filename()); if (rend_iter != this->tss_rendered_files.end()) { diff --git a/src/yajlpp/yajlpp.hh b/src/yajlpp/yajlpp.hh index 907db054..6ccd920f 100644 --- a/src/yajlpp/yajlpp.hh +++ b/src/yajlpp/yajlpp.hh @@ -488,6 +488,16 @@ public: return yajl_gen_integer(this->yg_handle, value); } + template + yajl_gen_status operator()(nonstd::optional value) + { + if (!value.has_value()) { + return yajl_gen_status_ok; + } + + return (*this)(value.value()); + } + template yajl_gen_status operator()( const T& container, diff --git a/src/yajlpp/yajlpp_def.hh b/src/yajlpp/yajlpp_def.hh index 796eadfd..2fc1c155 100644 --- a/src/yajlpp/yajlpp_def.hh +++ b/src/yajlpp/yajlpp_def.hh @@ -570,6 +570,12 @@ struct json_path_handler : public json_path_handler_base { = std::is_integral::value && !std::is_same::value; }; + template + struct LastIsNumber T::*> { + static constexpr bool value + = std::is_integral::value && !std::is_same::value; + }; + template struct LastIsVector { static constexpr bool value = LastIsVector::value; @@ -585,6 +591,18 @@ struct json_path_handler : public json_path_handler_base { static constexpr bool value = false; }; + template + static bool is_field_set(const nonstd::optional& field) + { + return field.has_value(); + } + + template + static bool is_field_set(const T&) + { + return true; + } + template::value, bool> = true> json_path_handler& for_field(Args... args) @@ -1153,6 +1171,10 @@ struct json_path_handler : public json_path_handler_base { } } + if (!is_field_set(field)) { + return yajl_gen_status_ok; + } + if (ygc.ygc_depth) { yajl_gen_string(handle, jph.jph_property); } diff --git a/test/test_logfile.sh b/test/test_logfile.sh index dfe96abc..0745dbf2 100644 --- a/test/test_logfile.sh +++ b/test/test_logfile.sh @@ -230,7 +230,6 @@ mv test_logfile.unreadable.out `test_err_filename` check_error_output "able to read an unreadable log file?" <