[RFC] Add a way to collect list usage statistics #7806

ZyX-I · 2018-01-03T01:40:36Z

This PR does two things:

Provides list length data in a number of cases (e.g. like getpos('.') knowing to return list with four items). This would be handy to reduce memory usage (and constant, a tiny bit) for lists populated from C code, and additionally allows to get statistics “how often such lists are actually edited in plugins”.
Adds code which allows to gather statistics about list usage. “Statistics” is actually a log to be processed later, guarded by LOG_LIST_ACTION define enabled by supplying -DLOG_LIST_ACTIONS=ON to cmake. Data is not written anywhere, staying in memory instead until user issues test_write_list_log('/path/to/list.log'). Should one attempt to write to an existing file it gets overwritten, log is erased from memory as it is written.

justinmk · 2018-01-09T08:47:18Z

CMakeLists.txt

@@ -277,6 +277,8 @@ else()
  set(DEBUG 0)
 endif()

+option(LOG_LIST_ACTIONS "Add list actions logging" OFF)


Should we make a habit of prefixing Nvim build options with NVIM_?

justinmk · 2018-01-09T08:49:18Z

src/nvim/eval/typval.h

+  /// All occurrences of this value should be eventually removed. This is for
+  /// the case when the only reason why list length is not known is that it
+  /// would be hard to code without refactoring, but refactoring is needed.
+  kListLenShouldKnow = -2,


If this might be removed in the future, it should be the last item.

justinmk · 2018-01-09T08:50:28Z

src/nvim/eval/typval.h

+extern ListLog *list_log_first;  ///< First list log chunk, NULL if missing
+extern ListLog *list_log_last;  ///< Last list log chunk
+
+static inline ListLog *list_log_alloc(const size_t size)


Most of our logging functions are prefixed with log_.

justinmk · 2018-01-09T08:51:47Z

Why a test_ viml function instead of double-underscore API function? I have been thinking of adding a nvim__stats API function for various internal statistics (such as statusline eval count).

ZyX-I · 2018-01-09T09:34:00Z

It appears that I have accidentally made empty output be represented by an empty list and not [''] which fails a bunch of job tests. Wondering whether [''] was intentional.

ZyX-I · 2018-01-09T09:39:07Z

Basically the reason is that buffer_to_tv_list is contained reimplementation of encode_list_write logic plus list allocation. I purged that out and used encode_list_write, but it thinks that empty output is fine to be represented by [], so just does nothing if len==0.

bfredl · 2018-01-09T11:37:02Z

[''] avoids special cases, which would be needed to deal with [], as it is logically more consistent (for instance, the number of \n is exactly the number of list elements minus one, no exceptions). Though for some reason I apparently forgot this case when implementing systemlist("true",1,1).

New logging is guarded by cmake LOG_LIST_ACTIONS define. To make it more efficient it is allocated as a linked list with chunks of length 2^(7+chunk_num); that uses basically the same idea as behind increasing kvec length (make appending O(1) (amortized)), but reduces constant by not bothering to move memory around what realloc() would surely do: it is not like we need random access to log entries here to justify usage of a single continuous memory block.

ZyX-I · 2018-01-14T13:20:09Z

For some reason travis is not running OS X builds for 14 hours already, but otherwise CI succeeded.

ZyX-I · 2018-01-16T10:05:49Z

Would be good to post stats from some typical editing sessions with statistics collection enabled.

justinmk · 2018-01-17T08:25:42Z

@ZyX-I here's a log
list.log

FEATURES: 3cc7ebf #7234 built-in VimL expression parser 6a7c904 #4419 implement <Cmd> key to invoke command in any mode b836328 #7679 'startup: treat stdin as text instead of commands' 58b210e :digraphs : highlight with hl-SpecialKey #2690 7a13611 #8276 'startup: Let `-s -` read from stdin' 1e71978 events: VimSuspend, VimResume #8280 1e7d5e8 #6272 'stdpath()' f96d99a #8247 server: introduce --listen e8c39f7 #8226 insert-mode: interpret unmapped META as ESC 98e7112 msg: do not scroll entire screen (#8088) f72630b #8055 let negative 'writedelay' show all redraws 5d2dd2e win: has("wsl") on Windows Subsystem for Linux #7330 a4f6cec cmdline: CmdlineEnter and CmdlineLeave autocommands (#7422) 207b7ca #6844 channels: support buffered output and bytes sockets/stdio API: f85cbea #7917 API: buffer updates 418abfc #6743 API: list information about all channels/jobs. 36b2e3f #8375 API: nvim_get_commands 273d2cd #8329 API: Make nvim_set_option() update `:verbose set …` 8d40b36 #8371 API: more reliable/descriptive VimL errors ebb1acb #8353 API: nvim_call_dict_function 9f994bb #8004 API: nvim_list_uis 3405704 #7520 API/UI: forward option updates to UIs 911b1e4 #7821 API: improve nvim_command_output WINDOWS OS: 9cefd83 #8084, #8516 build/win: support MSVC ee4e1fd win: Fix reading content from stdin (#8267) TUI: ffb8904 #8309 TUI: add support for mouse release events in urxvt 8d5a46e #8081 TUI: implement "standout" attribute 6071637 TUI: support TERM=konsole-256color 67848c0 #7653 TUI: report TUI info with -V3 ('verbose' >= 3) 3d0ee17 TUI/rxvt: enable focus-reporting d109f56 #7640 TUI: 'term' option: reflect effective terminal behavior FIXES: ed6a113 #8273 'job-control: avoid kill-timer race' 4e02f1a #8107 'jobs: separate process-group' 451c48a terminal: flush vterm output buffer on pty output #8486 5d6732f :checkhealth fixes #8335 53f11dc #8218 'Fix errors reported by PVS' d05712f inccommand: pause :terminal redraws (#8307) 51af911 inccommand: do not execute trailing commands #8256 84359a4 terminal: resize to the max dimensions (#8249) d49c1dd #8228 Make vim_fgets() return the same values as in Vim 60e96a4 screen: winhl=Normal:Background should not override syntax (#8093) 0c59ac1 #5908 'shada: Also save numbered marks' ba87a2c cscope: ignore EINTR while reading the prompt (#8079) b1412dc #7971 ':terminal Enter/Leave should not increment jumplist' 3a5721e TUI: libtermkey: force CSI driver for mouse input #7948 6ff13d7 #7720 TUI: faster startup 1c6e956 #7862 TUI: fix resize-related segfaults a58c909 #7676 TUI: always hide cursor when flushing, never flush buffers during unibilium output 303e1df #7624 TUI: disable BCE almost always 249bdb0 #7761 mark: Make sure that jumplist item will not have zero lnum 6f41ce0 #7704 macOS: Set $LANG based on the system locale a043899 #7633 'Retry fgets on EINTR' CHANGES: ad60927 #8304 default to 'nofsync' f3f1970 #8035 defaults: 'fillchars' a6052c7 #7984 defaults: sidescroll=1 b69fa86 #7888 defaults: enable cscopeverbose 7c4bb23 defaults: do :filetype stuff unless explicitly "off" 2aa308c #5658 'Apply :lmap in macros' 8ce6393 terminal: Leave 'relativenumber' alone (#8360) e46534b #4486 refactor: Remove maxmem, maxmemtot options 131aad9 win: defaults: 'shellcmdflag', 'shellxquote' #7343 c57d315 #8031 jobwait(): return -2 on interrupt also with timeout 6452831 clipboard: macOS: fallback to tmux if pbcopy is broken #7940 300d365 #7919 Make 'langnoremap' apply directly after a map ada1956 #7880 'lua/executor: Remove lightuserdata' INTERNAL: de0a954 #7806 internal statistics for list impl dee78a4 #7708 rewrite internal list impl

ZyX-I force-pushed the list-stat branch from bf820f2 to eda92a1 Compare January 3, 2018 02:06

marvim added the RFC label Jan 3, 2018

ZyX-I force-pushed the list-stat branch 2 times, most recently from e0877c6 to 834c981 Compare January 9, 2018 08:17

justinmk reviewed Jan 9, 2018

View reviewed changes

ZyX-I force-pushed the list-stat branch from 834c981 to 48c4577 Compare January 9, 2018 09:27

ZyX-I force-pushed the list-stat branch from 11f62d3 to 82d633c Compare January 9, 2018 09:40

ZyX-I mentioned this pull request Jan 10, 2018

test: fnamemodify() #7829

Closed

ZyX-I added 4 commits January 14, 2018 01:33

*: Provide list length when allocating lists

9ea1752

os/fileio: Fix some flag names in file_* functions documentation

c10ae4b

channel: Make empty output be represented by [''] again

a8cb510

ZyX-I force-pushed the list-stat branch from 82d633c to a8cb510 Compare January 13, 2018 22:33

justinmk merged commit de0a954 into neovim:master Jan 15, 2018

justinmk removed the RFC label Jan 15, 2018

ZyX-I deleted the list-stat branch January 15, 2018 23:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] Add a way to collect list usage statistics #7806

[RFC] Add a way to collect list usage statistics #7806

ZyX-I commented Jan 3, 2018

justinmk Jan 9, 2018

justinmk Jan 9, 2018

justinmk Jan 9, 2018

justinmk commented Jan 9, 2018

ZyX-I commented Jan 9, 2018

ZyX-I commented Jan 9, 2018 •

edited

Loading

bfredl commented Jan 9, 2018

ZyX-I commented Jan 14, 2018 •

edited

Loading

ZyX-I commented Jan 16, 2018

justinmk commented Jan 17, 2018

[RFC] Add a way to collect list usage statistics #7806

[RFC] Add a way to collect list usage statistics #7806

Conversation

ZyX-I commented Jan 3, 2018

justinmk Jan 9, 2018

Choose a reason for hiding this comment

justinmk Jan 9, 2018

Choose a reason for hiding this comment

justinmk Jan 9, 2018

Choose a reason for hiding this comment

justinmk commented Jan 9, 2018

ZyX-I commented Jan 9, 2018

ZyX-I commented Jan 9, 2018 • edited Loading

bfredl commented Jan 9, 2018

ZyX-I commented Jan 14, 2018 • edited Loading

ZyX-I commented Jan 16, 2018

justinmk commented Jan 17, 2018

ZyX-I commented Jan 9, 2018 •

edited

Loading

ZyX-I commented Jan 14, 2018 •

edited

Loading