From 043f85210a06168e36f103950897e00918504f6f Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Mon, 3 Oct 2016 10:46:11 +0200
Subject: [PATCH 1/5] tui: "backpressure": Drop messages to avoid flooding.

Closes #1234

multiqueue:
- Implement multiqueue_size()
- Rename MultiQueueItem.parent to MultiQueueItem.parent_item, to avoid confusion
  with MultiQueue.parent.
---
 src/nvim/event/loop.c                         | 16 ++++++
 src/nvim/event/multiqueue.c                   | 51 +++++++++++++------
 src/nvim/lib/queue.h                          | 18 +++----
 src/nvim/macros.h                             |  3 ++
 src/nvim/tui/input.c                          |  2 -
 src/nvim/tui/tui.c                            | 15 ++++++
 src/nvim/ui.c                                 |  7 ++-
 src/nvim/ui_bridge.c                          | 33 +++++++++---
 src/nvim/ui_bridge.h                          |  2 +-
 src/nvim/version.c                            |  4 +-
 .../{queue_spec.lua => multiqueue_spec.lua}   | 21 ++++++++
 11 files changed, 130 insertions(+), 42 deletions(-)
 rename test/unit/{queue_spec.lua => multiqueue_spec.lua} (81%)

diff --git a/src/nvim/event/loop.c b/src/nvim/event/loop.c
index d562ac1ed3..0e1775d01b 100644
--- a/src/nvim/event/loop.c
+++ b/src/nvim/event/loop.c
@@ -92,6 +92,22 @@ void loop_close(Loop *loop, bool wait)
   kl_destroy(WatcherPtr, loop->children);
 }
 
+void loop_purge(Loop *loop)
+{
+  uv_mutex_lock(&loop->mutex);
+  multiqueue_purge_events(loop->thread_events);
+  multiqueue_purge_events(loop->fast_events);
+  uv_mutex_unlock(&loop->mutex);
+}
+
+size_t loop_size(Loop *loop)
+{
+  uv_mutex_lock(&loop->mutex);
+  size_t rv = multiqueue_size(loop->thread_events);
+  uv_mutex_unlock(&loop->mutex);
+  return rv;
+}
+
 static void async_cb(uv_async_t *handle)
 {
   Loop *l = handle->loop->data;
diff --git a/src/nvim/event/multiqueue.c b/src/nvim/event/multiqueue.c
index 7efdfc4cad..79b4dd9458 100644
--- a/src/nvim/event/multiqueue.c
+++ b/src/nvim/event/multiqueue.c
@@ -1,6 +1,7 @@
-// Multi-level queue for selective async event processing. Multiqueue supports
-// a parent-child relationship with the following properties:
+// Multi-level queue for selective async event processing.
+// Not threadsafe; access must be synchronized externally.
 //
+// Multiqueue supports a parent-child relationship with these properties:
 // - pushing a node to a child queue will push a corresponding link node to the
 //   parent queue
 // - removing a link node from a parent queue will remove the next node
@@ -14,8 +15,7 @@
 //                         +----------------+
 //                         |   Main loop    |
 //                         +----------------+
-//                                  ^
-//                                  |
+//
 //                         +----------------+
 //         +-------------->|   Event loop   |<------------+
 //         |               +--+-------------+             |
@@ -60,7 +60,7 @@ struct multiqueue_item {
     MultiQueue *queue;
     struct {
       Event event;
-      MultiQueueItem *parent;
+      MultiQueueItem *parent_item;
     } item;
   } data;
   bool link;  // true: current item is just a link to a node in a child queue
@@ -69,9 +69,10 @@ struct multiqueue_item {
 
 struct multiqueue {
   MultiQueue *parent;
-  QUEUE headtail;
+  QUEUE headtail;  // circularly-linked
   put_callback put_cb;
   void *data;
+  size_t size;
 };
 
 #ifdef INCLUDE_GENERATED_DECLARATIONS
@@ -88,7 +89,8 @@ MultiQueue *multiqueue_new_parent(put_callback put_cb, void *data)
 MultiQueue *multiqueue_new_child(MultiQueue *parent)
   FUNC_ATTR_NONNULL_ALL
 {
-  assert(!parent->parent);
+  assert(!parent->parent);  // parent cannot have a parent, more like a "root"
+  parent->size++;
   return multiqueue_new(parent, NULL, NULL);
 }
 
@@ -97,6 +99,7 @@ static MultiQueue *multiqueue_new(MultiQueue *parent, put_callback put_cb,
 {
   MultiQueue *rv = xmalloc(sizeof(MultiQueue));
   QUEUE_INIT(&rv->headtail);
+  rv->size = 0;
   rv->parent = parent;
   rv->put_cb = put_cb;
   rv->data = data;
@@ -110,8 +113,8 @@ void multiqueue_free(MultiQueue *this)
     QUEUE *q = QUEUE_HEAD(&this->headtail);
     MultiQueueItem *item = multiqueue_node_data(q);
     if (this->parent) {
-      QUEUE_REMOVE(&item->data.item.parent->node);
-      xfree(item->data.item.parent);
+      QUEUE_REMOVE(&item->data.item.parent_item->node);
+      xfree(item->data.item.parent_item);
     }
     QUEUE_REMOVE(q);
     xfree(item);
@@ -145,6 +148,15 @@ void multiqueue_process_events(MultiQueue *this)
   }
 }
 
+/// Removes all events without processing them.
+void multiqueue_purge_events(MultiQueue *this)
+{
+  assert(this);
+  while (!multiqueue_empty(this)) {
+    (void)multiqueue_remove(this);
+  }
+}
+
 bool multiqueue_empty(MultiQueue *this)
 {
   assert(this);
@@ -157,6 +169,12 @@ void multiqueue_replace_parent(MultiQueue *this, MultiQueue *new_parent)
   this->parent = new_parent;
 }
 
+/// Gets the count of all events currently in the queue.
+size_t multiqueue_size(MultiQueue *this)
+{
+  return this->size;
+}
+
 static Event multiqueue_remove(MultiQueue *this)
 {
   assert(!multiqueue_empty(this));
@@ -178,12 +196,13 @@ static Event multiqueue_remove(MultiQueue *this)
   } else {
     if (this->parent) {
       // remove the corresponding link node in the parent queue
-      QUEUE_REMOVE(&item->data.item.parent->node);
-      xfree(item->data.item.parent);
+      QUEUE_REMOVE(&item->data.item.parent_item->node);
+      xfree(item->data.item.parent_item);
     }
     rv = item->data.item.event;
   }
 
+  this->size--;
   xfree(item);
   return rv;
 }
@@ -196,11 +215,13 @@ static void multiqueue_push(MultiQueue *this, Event event)
   QUEUE_INSERT_TAIL(&this->headtail, &item->node);
   if (this->parent) {
     // push link node to the parent queue
-    item->data.item.parent = xmalloc(sizeof(MultiQueueItem));
-    item->data.item.parent->link = true;
-    item->data.item.parent->data.queue = this;
-    QUEUE_INSERT_TAIL(&this->parent->headtail, &item->data.item.parent->node);
+    item->data.item.parent_item = xmalloc(sizeof(MultiQueueItem));
+    item->data.item.parent_item->link = true;
+    item->data.item.parent_item->data.queue = this;
+    QUEUE_INSERT_TAIL(&this->parent->headtail,
+                      &item->data.item.parent_item->node);
   }
+  this->size++;
 }
 
 static MultiQueueItem *multiqueue_node_data(QUEUE *q)
diff --git a/src/nvim/lib/queue.h b/src/nvim/lib/queue.h
index 9fcedf298f..ab9270081e 100644
--- a/src/nvim/lib/queue.h
+++ b/src/nvim/lib/queue.h
@@ -1,3 +1,8 @@
+// Queue implemented by circularly-linked list.
+//
+// Adapted from libuv. Simpler and more efficient than klist.h for implementing
+// queues that support arbitrary insertion/removal.
+//
 // Copyright (c) 2013, Ben Noordhuis <info@bnoordhuis.nl>
 //
 // Permission to use, copy, modify, and/or distribute this software for any
@@ -28,6 +33,8 @@ typedef struct _queue {
 #define QUEUE_DATA(ptr, type, field) \
   ((type *)((char *)(ptr) - offsetof(type, field)))
 
+// Important note: mutating the list while QUEUE_FOREACH is
+// iterating over its elements results in undefined behavior.
 #define QUEUE_FOREACH(q, h) \
   for (  /* NOLINT(readability/braces) */ \
       (q) = (h)->next; (q) != (h); (q) = (q)->next)
@@ -56,17 +63,6 @@ static inline void QUEUE_ADD(QUEUE *const h, QUEUE *const n)
   h->prev->next = h;
 }
 
-static inline void QUEUE_SPLIT(QUEUE *const h, QUEUE *const q, QUEUE *const n)
-  FUNC_ATTR_ALWAYS_INLINE
-{
-  n->prev = h->prev;
-  n->prev->next = n;
-  n->next = q;
-  h->prev = q->prev;
-  h->prev->next = h;
-  q->prev = n;
-}
-
 static inline void QUEUE_INSERT_HEAD(QUEUE *const h, QUEUE *const q)
   FUNC_ATTR_ALWAYS_INLINE
 {
diff --git a/src/nvim/macros.h b/src/nvim/macros.h
index 79e545771e..df2b431e92 100644
--- a/src/nvim/macros.h
+++ b/src/nvim/macros.h
@@ -158,4 +158,7 @@
 
 #define RGB(r, g, b) ((r << 16) | (g << 8) | b)
 
+#define STR_(x) #x
+#define STR(x) STR_(x)
+
 #endif  // NVIM_MACROS_H
diff --git a/src/nvim/tui/input.c b/src/nvim/tui/input.c
index 9dc66420b0..70d87a7ab2 100644
--- a/src/nvim/tui/input.c
+++ b/src/nvim/tui/input.c
@@ -319,8 +319,6 @@ static bool handle_forced_escape(TermInput *input)
   return false;
 }
 
-static void restart_reading(void **argv);
-
 static void read_cb(Stream *stream, RBuffer *buf, size_t c, void *data,
     bool eof)
 {
diff --git a/src/nvim/tui/tui.c b/src/nvim/tui/tui.c
index 2171e580ba..bceb4ca4ff 100644
--- a/src/nvim/tui/tui.c
+++ b/src/nvim/tui/tui.c
@@ -11,6 +11,7 @@
 #include "nvim/lib/kvec.h"
 
 #include "nvim/vim.h"
+#include "nvim/log.h"
 #include "nvim/ui.h"
 #include "nvim/map.h"
 #include "nvim/main.h"
@@ -32,6 +33,8 @@
 #define CNORM_COMMAND_MAX_SIZE 32
 #define OUTBUF_SIZE 0xffff
 
+#define TOO_MANY_EVENTS 1000000
+
 typedef struct {
   int top, bot, left, right;
 } Rect;
@@ -591,6 +594,18 @@ static void tui_flush(UI *ui)
   TUIData *data = ui->data;
   UGrid *grid = &data->grid;
 
+  size_t nrevents = loop_size(data->loop);
+  if (nrevents > TOO_MANY_EVENTS) {
+    ILOG("TUI event-queue flooded (thread_events=%zu); purging", nrevents);
+    // Back-pressure: UI events may accumulate much faster than the terminal
+    // device can serve them. Even if SIGINT/CTRL-C is received, user must still
+    // wait for the TUI event-queue to drain, and if there are ~millions of
+    // events in the queue, it could take hours. Clearing the queue allows the
+    // UI to recover. #1234 #5396
+    loop_purge(data->loop);
+    tui_busy_stop(ui);  // avoid hidden cursor
+  }
+
   while (kv_size(data->invalid_regions)) {
     Rect r = kv_pop(data->invalid_regions);
     int currow = -1;
diff --git a/src/nvim/ui.c b/src/nvim/ui.c
index ea0bccb1cd..d3784b6cd3 100644
--- a/src/nvim/ui.c
+++ b/src/nvim/ui.c
@@ -88,18 +88,17 @@ void ui_builtin_start(void)
 #ifdef FEAT_TUI
   tui_start();
 #else
-  fprintf(stderr, "Neovim was built without a Terminal UI," \
-          "press Ctrl+C to exit\n");
-
+  fprintf(stderr, "Nvim headless-mode started.\n");
   size_t len;
   char **addrs = server_address_list(&len);
   if (addrs != NULL) {
-    fprintf(stderr, "currently listening on the following address(es)\n");
+    fprintf(stderr, "Listening on:\n");
     for (size_t i = 0; i < len; i++) {
       fprintf(stderr, "\t%s\n", addrs[i]);
     }
     xfree(addrs);
   }
+  fprintf(stderr, "Press CTRL+C to exit.\n");
 #endif
 }
 
diff --git a/src/nvim/ui_bridge.c b/src/nvim/ui_bridge.c
index cc27c734e0..25861abc1b 100644
--- a/src/nvim/ui_bridge.c
+++ b/src/nvim/ui_bridge.c
@@ -1,11 +1,12 @@
-// UI wrapper that sends UI requests to the UI thread.
-// Used by the built-in TUI and external libnvim-based UIs.
+// UI wrapper that sends requests to the UI thread.
+// Used by the built-in TUI and libnvim-based UIs.
 
 #include <assert.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <limits.h>
 
+#include "nvim/log.h"
 #include "nvim/main.h"
 #include "nvim/vim.h"
 #include "nvim/ui.h"
@@ -19,10 +20,30 @@
 
 #define UI(b) (((UIBridgeData *)b)->ui)
 
-// Call a function in the UI thread
+#if MIN_LOG_LEVEL <= DEBUG_LOG_LEVEL
+static size_t        uilog_seen = 0;
+static argv_callback uilog_event = NULL;
+#define UI_CALL(ui, name, argc, ...) \
+  do { \
+    if (uilog_event == ui_bridge_##name##_event) { \
+      uilog_seen++; \
+    } else { \
+      if (uilog_seen > 0) { \
+        DLOG("UI bridge: ...%zu times", uilog_seen); \
+      } \
+      DLOG("UI bridge: " STR(name)); \
+      uilog_seen = 0; \
+      uilog_event = ui_bridge_##name##_event; \
+    } \
+    ((UIBridgeData *)ui)->scheduler( \
+        event_create(1, ui_bridge_##name##_event, argc, __VA_ARGS__), UI(ui)); \
+  } while (0)
+#else
+// Schedule a function call on the UI bridge thread.
 #define UI_CALL(ui, name, argc, ...) \
   ((UIBridgeData *)ui)->scheduler( \
       event_create(1, ui_bridge_##name##_event, argc, __VA_ARGS__), UI(ui))
+#endif
 
 #define INT2PTR(i) ((void *)(uintptr_t)i)
 #define PTR2INT(p) ((int)(uintptr_t)p)
@@ -218,16 +239,16 @@ static void ui_bridge_mode_change_event(void **argv)
 }
 
 static void ui_bridge_set_scroll_region(UI *b, int top, int bot, int left,
-    int right)
+                                        int right)
 {
   UI_CALL(b, set_scroll_region, 5, b, INT2PTR(top), INT2PTR(bot),
-      INT2PTR(left), INT2PTR(right));
+          INT2PTR(left), INT2PTR(right));
 }
 static void ui_bridge_set_scroll_region_event(void **argv)
 {
   UI *ui = UI(argv[0]);
   ui->set_scroll_region(ui, PTR2INT(argv[1]), PTR2INT(argv[2]),
-      PTR2INT(argv[3]), PTR2INT(argv[4]));
+                        PTR2INT(argv[3]), PTR2INT(argv[4]));
 }
 
 static void ui_bridge_scroll(UI *b, int count)
diff --git a/src/nvim/ui_bridge.h b/src/nvim/ui_bridge.h
index 9e4bf9f2a7..dba461550f 100644
--- a/src/nvim/ui_bridge.h
+++ b/src/nvim/ui_bridge.h
@@ -1,5 +1,5 @@
 // Bridge for communication between a UI thread and nvim core.
-// Used by the built-in TUI and external libnvim-based UIs.
+// Used by the built-in TUI and libnvim-based UIs.
 #ifndef NVIM_UI_BRIDGE_H
 #define NVIM_UI_BRIDGE_H
 
diff --git a/src/nvim/version.c b/src/nvim/version.c
index c48b26c9ce..10a25d5b8c 100644
--- a/src/nvim/version.c
+++ b/src/nvim/version.c
@@ -13,6 +13,7 @@
 #include "nvim/iconv.h"
 #include "nvim/version.h"
 #include "nvim/charset.h"
+#include "nvim/macros.h"
 #include "nvim/memline.h"
 #include "nvim/memory.h"
 #include "nvim/message.h"
@@ -22,9 +23,6 @@
 // version info generated by the build system
 #include "auto/versiondef.h"
 
-#define STR_(x) #x
-#define STR(x) STR_(x)
-
 // for ":version", ":intro", and "nvim --version"
 #ifndef NVIM_VERSION_MEDIUM
 #define NVIM_VERSION_MEDIUM STR(NVIM_VERSION_MAJOR) "." STR(NVIM_VERSION_MINOR)\
diff --git a/test/unit/queue_spec.lua b/test/unit/multiqueue_spec.lua
similarity index 81%
rename from test/unit/queue_spec.lua
rename to test/unit/multiqueue_spec.lua
index d802367835..c7f8dd8328 100644
--- a/test/unit/queue_spec.lua
+++ b/test/unit/multiqueue_spec.lua
@@ -36,6 +36,27 @@ describe("multiqueue (multi-level event-queue)", function()
     put(child3, 'c3i2')
   end)
 
+  it('keeps count of added events', function()
+    eq(3, multiqueue.multiqueue_size(child1))
+    eq(4, multiqueue.multiqueue_size(child2))
+    eq(2, multiqueue.multiqueue_size(child3))
+  end)
+
+  it('keeps count of removed events', function()
+    multiqueue.multiqueue_get(child1)
+    eq(2, multiqueue.multiqueue_size(child1))
+    multiqueue.multiqueue_get(child1)
+    eq(1, multiqueue.multiqueue_size(child1))
+    multiqueue.multiqueue_get(child1)
+    eq(0, multiqueue.multiqueue_size(child1))
+    put(child1, 'c2ixx')
+    eq(1, multiqueue.multiqueue_size(child1))
+    multiqueue.multiqueue_get(child1)
+    eq(0, multiqueue.multiqueue_size(child1))
+    multiqueue.multiqueue_get(child1)
+    eq(0, multiqueue.multiqueue_size(child1))
+  end)
+
   it('removing from parent removes from child', function()
     eq('c1i1', get(parent))
     eq('c1i2', get(parent))

From 97204e1cef4f922cc1f8e67f8a1f2f695d7da826 Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Fri, 30 Sep 2016 02:33:50 +0200
Subject: [PATCH 2/5] os/shell: Throttle :! output, pulse "..." message.

Periodically skip :! spam. This is a "cheat" that works for all UIs and greatly
improves responsiveness when :! spams MB or GB of output:
    :!yes
    :!while true; do date; done
    :!git grep ''
    :grep -r '' *

After ~10KB of data is seen from a single :! invocation, output will be skipped
for ~1s and three dots "..." will pulse in the bottom-left. Thereafter the
behavior alternates at every:
    * 10KB received
    * ~1s throttled

This also avoids out-of-memory which could happen with large :! outputs.

Note: This commit does not change the behavior of execute(':!foo').
      execute(':!foo') returns the string ':!foo^M', it captures *only* Vim
      messages, *not* shell command output. Vim behaves the same way.
      Use system('foo') for capturing shell command output.

Closes #1234

Helped-by: oni-link <knil.ino@gmail.com>
---
 runtime/doc/various.txt               | 12 +++-
 runtime/doc/vim_diff.txt              |  5 ++
 src/nvim/main.c                       |  2 +-
 src/nvim/os/shell.c                   | 91 ++++++++++++++++++++++++++-
 test/functional/eval/execute_spec.lua | 17 +++--
 test/functional/terminal/helpers.lua  |  1 +
 test/functional/ui/output_spec.lua    |  8 +++
 test/functional/ui/screen.lua         | 47 ++++++++++----
 8 files changed, 158 insertions(+), 25 deletions(-)

diff --git a/runtime/doc/various.txt b/runtime/doc/various.txt
index a1bf379d86..625416146e 100644
--- a/runtime/doc/various.txt
+++ b/runtime/doc/various.txt
@@ -255,14 +255,20 @@ g8			Print the hex values of the bytes used in the
 			backslashes are before the newline, only one is
 			removed.
 
-			On Unix the command normally runs in a non-interactive
-			shell.  If you want an interactive shell to be used
-			(to use aliases) set 'shellcmdflag' to "-ic".
+			The command runs in a non-interactive shell connected
+			to a pipe (not a terminal). Use |:terminal| to run an
+			interactive shell connected to a terminal.
+
 			For Win32 also see |:!start|.
 
 			After the command has been executed, the timestamp and
 			size of the current file is checked |timestamp|.
 
+			If the command produces a lot of output the displayed
+			output will be "throttled" so the command can execute
+			quickly without waiting for the display.  This only
+			affects the display, no data is lost.
+
 			Vim redraws the screen after the command is finished,
 			because it may have printed any text.  This requires a
 			hit-enter prompt, so that you can read any messages.
diff --git a/runtime/doc/vim_diff.txt b/runtime/doc/vim_diff.txt
index c4795bec57..1940bc55fa 100644
--- a/runtime/doc/vim_diff.txt
+++ b/runtime/doc/vim_diff.txt
@@ -155,6 +155,11 @@ are always available and may be used simultaneously in separate plugins.  The
 
 |system()| does not support writing/reading "backgrounded" commands. |E5677|
 
+Nvim truncates ("throttles") shell-command messages echoed by |:!|, |:grep|,
+and |:make|. No data is lost, this only affects display. This makes things
+faster, but may seem weird for commands like ":!cat foo". Use ":te cat foo"
+instead, |:terminal| output is never throttled.
+
 |mkdir()| behaviour changed:
 1. Assuming /tmp/foo does not exist and /tmp can be written to
    mkdir('/tmp/foo/bar', 'p', 0700) will create both /tmp/foo and /tmp/foo/bar 
diff --git a/src/nvim/main.c b/src/nvim/main.c
index 9b9976ac0a..12227565f3 100644
--- a/src/nvim/main.c
+++ b/src/nvim/main.c
@@ -534,7 +534,7 @@ int main(int argc, char **argv)
   }
 
   TIME_MSG("before starting main loop");
-  ILOG("Starting Neovim main loop.");
+  ILOG("starting main loop");
 
   /*
    * Call the main command loop.  This never returns.
diff --git a/src/nvim/os/shell.c b/src/nvim/os/shell.c
index 9e6effd21b..a300984f63 100644
--- a/src/nvim/os/shell.c
+++ b/src/nvim/os/shell.c
@@ -187,6 +187,8 @@ static int do_os_system(char **argv,
                         bool silent,
                         bool forward_output)
 {
+  out_data_throttle(NULL, 0);  // Initialize throttle for this shell command.
+
   // the output buffer
   DynamicBuffer buf = DYNAMIC_BUFFER_INIT;
   stream_read_cb data_cb = system_data_cb;
@@ -253,8 +255,8 @@ static int do_os_system(char **argv,
     wstream_set_write_cb(&in, shell_write_cb, NULL);
   }
 
-  // invoke busy_start here so event_poll_until wont change the busy state for
-  // the UI
+  // Invoke busy_start here so LOOP_PROCESS_EVENTS_UNTIL will not change the
+  // busy state.
   ui_busy_start();
   ui_flush();
   int status = process_wait(proc, -1, NULL);
@@ -309,6 +311,86 @@ static void system_data_cb(Stream *stream, RBuffer *buf, size_t count,
   dbuf->len += nread;
 }
 
+/// Tracks output received for the current executing shell command. To avoid
+/// flooding the UI, output is periodically skipped and a pulsing "..." is
+/// shown instead. Tracking depends on the synchronous/blocking nature of ":!".
+//
+/// Purpose:
+///   1. CTRL-C is more responsive. #1234 #5396
+///   2. Improves performance of :! (UI, esp. TUI, is the bottleneck here).
+///   3. Avoids OOM during long-running, spammy :!.
+///
+/// Note:
+///   - Throttling "solves" the issue for *all* UIs, on all platforms.
+///   - Unlikely that users will miss useful output: humans do not read >100KB.
+///   - Caveat: Affects execute(':!foo'), but that is not a "very important"
+///     case; system('foo') should be used for large outputs.
+///
+/// Vim does not need this hack because:
+///   1. :! in terminal-Vim runs in cooked mode, so CTRL-C is caught by the
+///      terminal and raises SIGINT out-of-band.
+///   2. :! in terminal-Vim uses a tty (Nvim uses pipes), so commands
+///      (e.g. `git grep`) may page themselves.
+///
+/// @returns true if output was skipped and pulse was displayed
+static bool out_data_throttle(char *output, size_t size)
+{
+#define NS_1_SECOND 1000000000        // 1s, in ns
+#define THRESHOLD   1024 * 10         // 10KB, "a few screenfuls" of data.
+  static uint64_t   started     = 0;  // Start time of the current throttle.
+  static size_t     received    = 0;  // Bytes observed since last throttle.
+  static size_t     visit       = 0;  // "Pulse" count of the current throttle.
+  static size_t     max_visits  = 0;
+  static char       pulse_msg[] = { ' ', ' ', ' ', '\0' };
+
+  if (output == NULL) {
+    started = received = visit = 0;
+    max_visits = 10;
+    return false;
+  }
+
+  received += size;
+  if (received < THRESHOLD
+      // Display at least the first chunk of output even if it is >=THRESHOLD.
+      || (!started && received < size + 1000)) {
+    return false;
+  }
+
+  if (!visit) {
+    started = os_hrtime();
+  }
+
+  if (visit >= max_visits) {
+    uint64_t since = os_hrtime() - started;
+    if (since < NS_1_SECOND) {
+      // Adjust max_visits based on the current relative performance.
+      // Each "pulse" period should last >=1 second so that it is perceptible.
+      max_visits = (2 * max_visits);
+    } else {
+      received = visit = 0;
+    }
+  }
+
+  if (received && ++visit <= max_visits) {
+    // Pulse "..." at the bottom of the screen.
+    size_t tick = (visit == max_visits)
+                  ? 3  // Force all dots "..." on last visit.
+                  : (visit + 1) % 4;
+    pulse_msg[0] = (tick == 0) ? ' ' : '.';
+    pulse_msg[1] = (tick == 0 || 1 == tick) ? ' ' : '.';
+    pulse_msg[2] = (tick == 0 || 1 == tick || 2 == tick) ? ' ' : '.';
+    if (visit == 1) {
+      screen_del_lines(0, 0, 1, (int)Rows, NULL);
+    }
+    int lastrow = (int)Rows - 1;
+    screen_puts_len((char_u *)pulse_msg, ARRAY_SIZE(pulse_msg), lastrow, 0, 0);
+    ui_flush();
+    return true;
+  }
+
+  return false;
+}
+
 /// Continue to append data to last screen line.
 ///
 /// @param output       Data to append to screen lines.
@@ -319,6 +401,11 @@ static void append_to_screen_end(char *output, size_t remaining, bool new_line)
   // Column of last row to start appending data to.
   static colnr_T last_col = 0;
 
+  if (out_data_throttle(output, remaining)) {
+    last_col = 0;
+    return;
+  }
+
   size_t off = 0;
   int last_row = (int)Rows - 1;
 
diff --git a/test/functional/eval/execute_spec.lua b/test/functional/eval/execute_spec.lua
index b5b481435a..fc13c0a72b 100644
--- a/test/functional/eval/execute_spec.lua
+++ b/test/functional/eval/execute_spec.lua
@@ -12,16 +12,16 @@ local feed = helpers.feed
 describe('execute()', function()
   before_each(clear)
 
-  it('returns the same result with :redir', function()
+  it('captures the same result as :redir', function()
     eq(redir_exec('messages'), funcs.execute('messages'))
   end)
 
-  it('returns the output of the commands if the argument is List', function()
+  it('captures the concatenated outputs of a List of commands', function()
     eq("foobar", funcs.execute({'echon "foo"', 'echon "bar"'}))
     eq("\nfoo\nbar", funcs.execute({'echo "foo"', 'echo "bar"'}))
   end)
 
-  it('supports the nested redirection', function()
+  it('supports nested redirection', function()
     source([[
     function! g:Foo()
       let a = ''
@@ -43,17 +43,17 @@ describe('execute()', function()
     eq('42', funcs.execute([[echon execute("echon execute('echon 42')")]]))
   end)
 
-  it('returns the transformed string', function()
+  it('captures a transformed string', function()
     eq('^A', funcs.execute('echon "\\<C-a>"'))
   end)
 
-  it('returns the empty string if the argument list is empty', function()
+  it('returns empty string if the argument list is empty', function()
     eq('', funcs.execute({}))
     eq(0, exc_exec('let g:ret = execute(v:_null_list)'))
     eq('', eval('g:ret'))
   end)
 
-  it('returns the errors', function()
+  it('captures errors', function()
     local ret
     ret = exc_exec('call execute(0.0)')
     eq('Vim(call):E806: using Float as a String', ret)
@@ -69,6 +69,11 @@ describe('execute()', function()
     eq('Vim(call):E729: using Funcref as a String', ret)
   end)
 
+  -- This matches Vim behavior.
+  it('does not capture shell-command output', function()
+    eq('\n:!echo "foo"\13\n', funcs.execute('!echo "foo"'))
+  end)
+
   it('silences command run inside', function()
     local screen = Screen.new(40, 5)
     screen:attach()
diff --git a/test/functional/terminal/helpers.lua b/test/functional/terminal/helpers.lua
index aacf109f2f..ae5e6d4b1f 100644
--- a/test/functional/terminal/helpers.lua
+++ b/test/functional/terminal/helpers.lua
@@ -51,6 +51,7 @@ local function screen_setup(extra_height, command)
     [7] = {foreground = 130},
     [8] = {foreground = 15, background = 1}, -- error message
     [9] = {foreground = 4},
+    [10] = {foreground = 2},  -- "Press ENTER" in embedded :terminal session.
   })
 
   screen:attach({rgb=false})
diff --git a/test/functional/ui/output_spec.lua b/test/functional/ui/output_spec.lua
index c58bbe9147..4aae2edfaa 100644
--- a/test/functional/ui/output_spec.lua
+++ b/test/functional/ui/output_spec.lua
@@ -1,5 +1,6 @@
 local session = require('test.functional.helpers')(after_each)
 local child_session = require('test.functional.terminal.helpers')
+local Screen = require('test.functional.ui.screen')
 
 if session.pending_win32(pending) then return end
 
@@ -39,4 +40,11 @@ describe("shell command :!", function()
       {3:-- TERMINAL --}                                    |
     ]])
   end)
+
+  it("throttles shell-command output greater than ~20KB", function()
+    child_session.feed_data(
+      ":!for i in $(seq 2 3000); do echo XXXXXXXXXX; done\n")
+    -- If a line with only a dot "." appears, then throttling was triggered.
+    screen:expect("\n.", nil, nil, nil, true)
+  end)
 end)
diff --git a/test/functional/ui/screen.lua b/test/functional/ui/screen.lua
index ebe8af35eb..2581b36711 100644
--- a/test/functional/ui/screen.lua
+++ b/test/functional/ui/screen.lua
@@ -126,7 +126,7 @@ end
 do
   local spawn, nvim_prog = helpers.spawn, helpers.nvim_prog
   local session = spawn({nvim_prog, '-u', 'NONE', '-i', 'NONE', '-N', '--embed'})
-  local status, rv = session:request('vim_get_color_map')
+  local status, rv = session:request('nvim_get_color_map')
   if not status then
     print('failed to get color map')
     os.exit(1)
@@ -207,7 +207,15 @@ function Screen:try_resize(columns, rows)
   uimeths.try_resize(columns, rows)
 end
 
-function Screen:expect(expected, attr_ids, attr_ignore, condition)
+-- Asserts that `expected` eventually matches the screen state.
+--
+-- expected:    Expected screen state (string).
+-- attr_ids:    Text attribute definitions.
+-- attr_ignore: Ignored text attributes.
+-- condition:   Function asserting some arbitrary condition.
+-- any:         true: Succeed if `expected` matches ANY screen line(s).
+--              false (default): `expected` must match screen exactly.
+function Screen:expect(expected, attr_ids, attr_ignore, condition, any)
   -- remove the last line and dedent
   expected = dedent(expected:gsub('\n[ ]+$', ''))
   local expected_rows = {}
@@ -229,21 +237,34 @@ function Screen:expect(expected, attr_ids, attr_ignore, condition)
     for i = 1, self._height do
       actual_rows[i] = self:_row_repr(self._rows[i], ids, ignore)
     end
-    for i = 1, self._height do
-      if expected_rows[i] ~= actual_rows[i] then
-        local msg_expected_rows = {}
-        for j = 1, #expected_rows do
-          msg_expected_rows[j] = expected_rows[j]
-        end
-        msg_expected_rows[i] = '*' .. msg_expected_rows[i]
-        actual_rows[i] = '*' .. actual_rows[i]
+
+    if any then
+      -- Search for `expected` anywhere in the screen lines.
+      local actual_screen_str = table.concat(actual_rows, '\n')
+      if nil == string.find(actual_screen_str, expected) then
         return (
-          'Row ' .. tostring(i) .. ' didn\'t match.\n'
-          .. 'Expected:\n|' .. table.concat(msg_expected_rows, '|\n|') .. '|\n'
-          .. 'Actual:\n|' .. table.concat(actual_rows, '|\n|') .. '|\n\n' .. [[
+          'Failed to match any screen lines.\n'
+          .. 'Expected (anywhere): "' .. expected .. '"\n'
+          .. 'Actual:\n  |' .. table.concat(actual_rows, '|\n  |') .. '|\n\n')
+      end
+    else
+      -- `expected` must match the screen lines exactly.
+      for i = 1, self._height do
+        if expected_rows[i] ~= actual_rows[i] then
+          local msg_expected_rows = {}
+          for j = 1, #expected_rows do
+            msg_expected_rows[j] = expected_rows[j]
+          end
+          msg_expected_rows[i] = '*' .. msg_expected_rows[i]
+          actual_rows[i] = '*' .. actual_rows[i]
+          return (
+            'Row ' .. tostring(i) .. ' did not match.\n'
+            ..'Expected:\n  |'..table.concat(msg_expected_rows, '|\n  |')..'|\n'
+            ..'Actual:\n  |'..table.concat(actual_rows, '|\n  |')..'|\n\n'..[[
 To print the expect() call that would assert the current screen state, use
 screen:snaphot_util(). In case of non-deterministic failures, use
 screen:redraw_debug() to show all intermediate screen states.  ]])
+        end
       end
     end
   end)

From cb589990079a0e58bed3e1831f70baaba52f66fc Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Fri, 7 Oct 2016 13:18:24 +0200
Subject: [PATCH 3/5] os/shell: do_os_system(): Always show last chunk.

This ameliorates use-cases like:
    :!cat foo.txt
    :make
where the user is interested in the last few lines of output.

Try these shell-based ex-commands before/after this commit:
    :grep -r '' *
    :make
    :!yes
    :!grep -r '' *
    :!git grep ''
    :!cat foo
    :!echo foo
    :!while true; do date; done
    :!for i in `seq 1 20000`; do echo XXXXXXXXXX $i; done

In all cases the last few lines of the command should always be shown,
regardless of where throttling was triggered.
---
 runtime/doc/various.txt            |   8 +-
 runtime/doc/vim_diff.txt           |   7 +-
 src/nvim/memory.c                  |  31 +++---
 src/nvim/os/shell.c                | 156 +++++++++++++++++++----------
 test/functional/ui/output_spec.lua |  21 +++-
 5 files changed, 141 insertions(+), 82 deletions(-)

diff --git a/runtime/doc/various.txt b/runtime/doc/various.txt
index 625416146e..3c1472446d 100644
--- a/runtime/doc/various.txt
+++ b/runtime/doc/various.txt
@@ -264,10 +264,10 @@ g8			Print the hex values of the bytes used in the
 			After the command has been executed, the timestamp and
 			size of the current file is checked |timestamp|.
 
-			If the command produces a lot of output the displayed
-			output will be "throttled" so the command can execute
-			quickly without waiting for the display.  This only
-			affects the display, no data is lost.
+			If the command produces too much output some lines may
+			be skipped so the command can execute quickly.  No
+			data is lost, this only affects the display.  The last
+			few lines are always displayed (never skipped).
 
 			Vim redraws the screen after the command is finished,
 			because it may have printed any text.  This requires a
diff --git a/runtime/doc/vim_diff.txt b/runtime/doc/vim_diff.txt
index 1940bc55fa..7ccdfd2bdd 100644
--- a/runtime/doc/vim_diff.txt
+++ b/runtime/doc/vim_diff.txt
@@ -155,10 +155,9 @@ are always available and may be used simultaneously in separate plugins.  The
 
 |system()| does not support writing/reading "backgrounded" commands. |E5677|
 
-Nvim truncates ("throttles") shell-command messages echoed by |:!|, |:grep|,
-and |:make|. No data is lost, this only affects display. This makes things
-faster, but may seem weird for commands like ":!cat foo". Use ":te cat foo"
-instead, |:terminal| output is never throttled.
+Nvim may throttle (skip) messages from shell commands (|:!|, |:grep|, |:make|)
+if there is too much output. No data is lost, this only affects display and
+makes things faster. |:terminal| output is never throttled.
 
 |mkdir()| behaviour changed:
 1. Assuming /tmp/foo does not exist and /tmp can be written to
diff --git a/src/nvim/memory.c b/src/nvim/memory.c
index 3e041be4d3..071ef335cf 100644
--- a/src/nvim/memory.c
+++ b/src/nvim/memory.c
@@ -283,18 +283,16 @@ size_t memcnt(const void *data, char c, size_t len)
   return cnt;
 }
 
-/// The xstpcpy() function shall copy the string pointed to by src (including
-/// the terminating NUL character) into the array pointed to by dst.
+/// Copies the string pointed to by src (including the terminating NUL
+/// character) into the array pointed to by dst.
 ///
-/// The xstpcpy() function shall return a pointer to the terminating NUL
-/// character copied into the dst buffer. This is the only difference with
-/// strcpy(), which returns dst.
+/// @returns pointer to the terminating NUL char copied into the dst buffer.
+///          This is the only difference with strcpy(), which returns dst.
 ///
-/// WARNING: If copying takes place between objects that overlap, the behavior is
-/// undefined.
+/// WARNING: If copying takes place between objects that overlap, the behavior
+/// is undefined.
 ///
-/// This is the Neovim version of stpcpy(3) as defined in POSIX 2008. We
-/// don't require that supported platforms implement POSIX 2008, so we
+/// Nvim version of POSIX 2008 stpcpy(3). We do not require POSIX 2008, so
 /// implement our own version.
 ///
 /// @param dst
@@ -306,16 +304,15 @@ char *xstpcpy(char *restrict dst, const char *restrict src)
   return (char *)memcpy(dst, src, len + 1) + len;
 }
 
-/// The xstpncpy() function shall copy not more than n bytes (bytes that follow
-/// a NUL character are not copied) from the array pointed to by src to the
-/// array pointed to by dst.
+/// Copies not more than n bytes (bytes that follow a NUL character are not
+/// copied) from the array pointed to by src to the array pointed to by dst.
 ///
-/// If a NUL character is written to the destination, the xstpncpy() function
-/// shall return the address of the first such NUL character. Otherwise, it
-/// shall return &dst[maxlen].
+/// If a NUL character is written to the destination, xstpncpy() returns the
+/// address of the first such NUL character. Otherwise, it shall return
+/// &dst[maxlen].
 ///
-/// WARNING: If copying takes place between objects that overlap, the behavior is
-/// undefined.
+/// WARNING: If copying takes place between objects that overlap, the behavior
+/// is undefined.
 ///
 /// WARNING: xstpncpy will ALWAYS write maxlen bytes. If src is shorter than
 /// maxlen, zeroes will be written to the remaining bytes.
diff --git a/src/nvim/os/shell.c b/src/nvim/os/shell.c
index a300984f63..f7325b20f2 100644
--- a/src/nvim/os/shell.c
+++ b/src/nvim/os/shell.c
@@ -25,7 +25,9 @@
 #include "nvim/charset.h"
 #include "nvim/strings.h"
 
-#define DYNAMIC_BUFFER_INIT {NULL, 0, 0}
+#define DYNAMIC_BUFFER_INIT { NULL, 0, 0 }
+#define NS_1_SECOND         1000000000      // 1 second, in nanoseconds
+#define OUT_DATA_THRESHOLD  1024 * 10U      // 10KB, "a few screenfuls" of data.
 
 typedef struct {
   char *data;
@@ -187,7 +189,8 @@ static int do_os_system(char **argv,
                         bool silent,
                         bool forward_output)
 {
-  out_data_throttle(NULL, 0);  // Initialize throttle for this shell command.
+  out_data_decide_throttle(0);  // Initialize throttle decider.
+  out_data_ring(NULL, 0);       // Initialize output ring-buffer.
 
   // the output buffer
   DynamicBuffer buf = DYNAMIC_BUFFER_INIT;
@@ -217,7 +220,7 @@ static int do_os_system(char **argv,
   proc->err = &err;
   if (!process_spawn(proc)) {
     loop_poll_events(&main_loop, 0);
-    // Failed, probably due to `sh` not being executable
+    // Failed, probably due to 'sh' not being executable
     if (!silent) {
       MSG_PUTS(_("\nCannot execute "));
       msg_outtrans((char_u *)prog);
@@ -260,6 +263,10 @@ static int do_os_system(char **argv,
   ui_busy_start();
   ui_flush();
   int status = process_wait(proc, -1, NULL);
+  if (!got_int && out_data_decide_throttle(0)) {
+    // Last chunk of output was skipped; display it now.
+    out_data_ring(NULL, SIZE_MAX);
+  }
   ui_busy_stop();
 
   // prepare the out parameters if requested
@@ -311,56 +318,50 @@ static void system_data_cb(Stream *stream, RBuffer *buf, size_t count,
   dbuf->len += nread;
 }
 
-/// Tracks output received for the current executing shell command. To avoid
-/// flooding the UI, output is periodically skipped and a pulsing "..." is
-/// shown instead. Tracking depends on the synchronous/blocking nature of ":!".
+/// Tracks output received for the current executing shell command, and displays
+/// a pulsing "..." when output should be skipped. Tracking depends on the
+/// synchronous/blocking nature of ":!".
 //
 /// Purpose:
 ///   1. CTRL-C is more responsive. #1234 #5396
-///   2. Improves performance of :! (UI, esp. TUI, is the bottleneck here).
+///   2. Improves performance of :! (UI, esp. TUI, is the bottleneck).
 ///   3. Avoids OOM during long-running, spammy :!.
 ///
-/// Note:
-///   - Throttling "solves" the issue for *all* UIs, on all platforms.
-///   - Unlikely that users will miss useful output: humans do not read >100KB.
-///   - Caveat: Affects execute(':!foo'), but that is not a "very important"
-///     case; system('foo') should be used for large outputs.
-///
 /// Vim does not need this hack because:
 ///   1. :! in terminal-Vim runs in cooked mode, so CTRL-C is caught by the
 ///      terminal and raises SIGINT out-of-band.
 ///   2. :! in terminal-Vim uses a tty (Nvim uses pipes), so commands
 ///      (e.g. `git grep`) may page themselves.
 ///
-/// @returns true if output was skipped and pulse was displayed
-static bool out_data_throttle(char *output, size_t size)
+/// @param size Length of data, used with internal state to decide whether
+///             output should be skipped. size=0 resets the internal state and
+///             returns the previous decision.
+///
+/// @returns true if output should be skipped and pulse was displayed.
+///          Returns the previous decision if size=0.
+static bool out_data_decide_throttle(size_t size)
 {
-#define NS_1_SECOND 1000000000        // 1s, in ns
-#define THRESHOLD   1024 * 10         // 10KB, "a few screenfuls" of data.
   static uint64_t   started     = 0;  // Start time of the current throttle.
   static size_t     received    = 0;  // Bytes observed since last throttle.
   static size_t     visit       = 0;  // "Pulse" count of the current throttle.
   static size_t     max_visits  = 0;
   static char       pulse_msg[] = { ' ', ' ', ' ', '\0' };
 
-  if (output == NULL) {
+  if (!size) {
+    bool previous_decision = (visit > 0); // TODO: needs to check that last print shows more than a page
     started = received = visit = 0;
-    max_visits = 10;
-    return false;
+    max_visits = 20;
+    return previous_decision;
   }
 
   received += size;
-  if (received < THRESHOLD
-      // Display at least the first chunk of output even if it is >=THRESHOLD.
+  if (received < OUT_DATA_THRESHOLD
+      // Display at least the first chunk of output even if it is big.
       || (!started && received < size + 1000)) {
     return false;
-  }
-
-  if (!visit) {
+  } else if (!visit) {
     started = os_hrtime();
-  }
-
-  if (visit >= max_visits) {
+  } else if (visit >= max_visits) {
     uint64_t since = os_hrtime() - started;
     if (since < NS_1_SECOND) {
       // Adjust max_visits based on the current relative performance.
@@ -368,27 +369,74 @@ static bool out_data_throttle(char *output, size_t size)
       max_visits = (2 * max_visits);
     } else {
       received = visit = 0;
+      return false;
     }
   }
 
-  if (received && ++visit <= max_visits) {
-    // Pulse "..." at the bottom of the screen.
-    size_t tick = (visit == max_visits)
-                  ? 3  // Force all dots "..." on last visit.
-                  : (visit + 1) % 4;
-    pulse_msg[0] = (tick == 0) ? ' ' : '.';
-    pulse_msg[1] = (tick == 0 || 1 == tick) ? ' ' : '.';
-    pulse_msg[2] = (tick == 0 || 1 == tick || 2 == tick) ? ' ' : '.';
-    if (visit == 1) {
-      screen_del_lines(0, 0, 1, (int)Rows, NULL);
-    }
-    int lastrow = (int)Rows - 1;
-    screen_puts_len((char_u *)pulse_msg, ARRAY_SIZE(pulse_msg), lastrow, 0, 0);
-    ui_flush();
-    return true;
+  visit++;
+  // Pulse "..." at the bottom of the screen.
+  size_t tick = (visit == max_visits)
+                ? 3  // Force all dots "..." on last visit.
+                : (visit % 4);
+  pulse_msg[0] = (tick == 0) ? ' ' : '.';
+  pulse_msg[1] = (tick == 0 || 1 == tick) ? ' ' : '.';
+  pulse_msg[2] = (tick == 0 || 1 == tick || 2 == tick) ? ' ' : '.';
+  if (visit == 1) {
+    screen_del_lines(0, 0, 1, (int)Rows, NULL);
+  }
+  int lastrow = (int)Rows - 1;
+  screen_puts_len((char_u *)pulse_msg, ARRAY_SIZE(pulse_msg), lastrow, 0, 0);
+  ui_flush();
+  return true;
+}
+
+/// Saves output in a quasi-ringbuffer. Used to ensure the last ~page of
+/// output for a shell-command is always displayed.
+///
+/// Init mode: Resets the internal state.
+///   output = NULL
+///   size   = 0
+/// Print mode: Displays the current saved data.
+///   output = NULL
+///   size   = SIZE_MAX
+///
+/// @param  output  Data to save, or NULL to invoke a special mode.
+/// @param  size    Length of `output`.
+static void out_data_ring(char *output, size_t size)
+{
+#define MAX_CHUNK_SIZE (OUT_DATA_THRESHOLD / 2)
+  static char    last_skipped[MAX_CHUNK_SIZE];  // Saved output.
+  static size_t  last_skipped_len = 0;
+
+  assert(output != NULL || (size == 0 || size == SIZE_MAX));
+
+  if (output == NULL && size == 0) {          // Init mode
+    last_skipped_len = 0;
+    return;
   }
 
-  return false;
+  if (output == NULL && size == SIZE_MAX) {   // Print mode
+    out_data_append_to_screen(last_skipped, last_skipped_len, true);
+    return;
+  }
+
+  // This is basically a ring-buffer...
+  if (size >= MAX_CHUNK_SIZE) {               // Save mode
+    size_t start = size - MAX_CHUNK_SIZE;
+    memcpy(last_skipped, output + start, MAX_CHUNK_SIZE);
+    last_skipped_len = MAX_CHUNK_SIZE;
+  } else if (size > 0) {
+    // Length of the old data that can be kept.
+    size_t keep_len   = MIN(last_skipped_len, MAX_CHUNK_SIZE - size);
+    size_t keep_start = last_skipped_len - keep_len;
+    // Shift the kept part of the old data to the start.
+    if (keep_start) {
+      memmove(last_skipped, last_skipped + keep_start, keep_len);
+    }
+    // Copy the entire new data to the remaining space.
+    memcpy(last_skipped + keep_len, output, size);
+    last_skipped_len = keep_len + size;
+  }
 }
 
 /// Continue to append data to last screen line.
@@ -396,15 +444,10 @@ static bool out_data_throttle(char *output, size_t size)
 /// @param output       Data to append to screen lines.
 /// @param remaining    Size of data.
 /// @param new_line     If true, next data output will be on a new line.
-static void append_to_screen_end(char *output, size_t remaining, bool new_line)
+static void out_data_append_to_screen(char *output, size_t remaining,
+                                      bool new_line)
 {
-  // Column of last row to start appending data to.
-  static colnr_T last_col = 0;
-
-  if (out_data_throttle(output, remaining)) {
-    last_col = 0;
-    return;
-  }
+  static colnr_T last_col = 0;  // Column of last row to append to.
 
   size_t off = 0;
   int last_row = (int)Rows - 1;
@@ -457,7 +500,14 @@ static void out_data_cb(Stream *stream, RBuffer *buf, size_t count, void *data,
   size_t cnt;
   char *ptr = rbuffer_read_ptr(buf, &cnt);
 
-  append_to_screen_end(ptr, cnt, eof);
+  if (ptr != NULL && cnt > 0
+      && out_data_decide_throttle(cnt)) {  // Skip output above a threshold.
+    // Save the skipped output. If it is the final chunk, we display it later.
+    out_data_ring(ptr, cnt);
+  } else {
+    out_data_append_to_screen(ptr, cnt, eof);
+  }
+
   if (cnt) {
     rbuffer_consumed(buf, cnt);
   }
diff --git a/test/functional/ui/output_spec.lua b/test/functional/ui/output_spec.lua
index 4aae2edfaa..d6d8f1c4e5 100644
--- a/test/functional/ui/output_spec.lua
+++ b/test/functional/ui/output_spec.lua
@@ -1,6 +1,5 @@
 local session = require('test.functional.helpers')(after_each)
 local child_session = require('test.functional.terminal.helpers')
-local Screen = require('test.functional.ui.screen')
 
 if session.pending_win32(pending) then return end
 
@@ -41,10 +40,24 @@ describe("shell command :!", function()
     ]])
   end)
 
-  it("throttles shell-command output greater than ~20KB", function()
+  it("throttles shell-command output greater than ~10KB", function()
+    screen.timeout = 20000  -- Avoid false failure on slow systems.
     child_session.feed_data(
-      ":!for i in $(seq 2 3000); do echo XXXXXXXXXX; done\n")
-    -- If a line with only a dot "." appears, then throttling was triggered.
+      ":!for i in $(seq 2 3000); do echo XXXXXXXXXX $i; done\n")
+
+    -- If we observe any line starting with a dot, then throttling occurred.
     screen:expect("\n.", nil, nil, nil, true)
+
+    -- Final chunk of output should always be displayed, never skipped.
+    -- (Throttling is non-deterministic, this test is merely a sanity check.)
+    screen:expect([[
+      XXXXXXXXXX 2996                                   |
+      XXXXXXXXXX 2997                                   |
+      XXXXXXXXXX 2998                                   |
+      XXXXXXXXXX 2999                                   |
+      XXXXXXXXXX 3000                                   |
+      {10:Press ENTER or type command to continue}{1: }          |
+      {3:-- TERMINAL --}                                    |
+    ]])
   end)
 end)

From ea154dfdb233db3e78a0347a7435ff4d547934a5 Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Sat, 8 Oct 2016 16:03:26 +0200
Subject: [PATCH 4/5] out_data_decide_throttle(): Avoid too-small final chunk.

---
 src/nvim/os/shell.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/nvim/os/shell.c b/src/nvim/os/shell.c
index f7325b20f2..d3fd4f87fb 100644
--- a/src/nvim/os/shell.c
+++ b/src/nvim/os/shell.c
@@ -348,7 +348,7 @@ static bool out_data_decide_throttle(size_t size)
   static char       pulse_msg[] = { ' ', ' ', ' ', '\0' };
 
   if (!size) {
-    bool previous_decision = (visit > 0); // TODO: needs to check that last print shows more than a page
+    bool previous_decision = (visit > 0);
     started = received = visit = 0;
     max_visits = 20;
     return previous_decision;
@@ -361,6 +361,10 @@ static bool out_data_decide_throttle(size_t size)
     return false;
   } else if (!visit) {
     started = os_hrtime();
+  } else if (visit >= max_visits && size < 256 && max_visits < 999) {
+    // Gobble up small chunks even if we maxed out. Avoids the case where the
+    // final displayed chunk is very tiny.
+    max_visits = visit + 1;
   } else if (visit >= max_visits) {
     uint64_t since = os_hrtime() - started;
     if (since < NS_1_SECOND) {

From 4abe9afbf67cec620fde9e47cb3df92f60e1cca9 Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Fri, 21 Oct 2016 17:01:36 +0200
Subject: [PATCH 5/5] out_data_decide_throttle(): timeout instead of hard
 limit.

Instead of managing max_visits, check the time every N visits. This avoids edge
cases where max_visits is large but the chunk frequency slowed down, which would
causing the "..." pulse to show for a very long time. It's more important to
show output at reasonable intervals than to avoid calling os_hrtime().
---
 src/nvim/os/shell.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/src/nvim/os/shell.c b/src/nvim/os/shell.c
index d3fd4f87fb..9514936ad0 100644
--- a/src/nvim/os/shell.c
+++ b/src/nvim/os/shell.c
@@ -26,7 +26,7 @@
 #include "nvim/strings.h"
 
 #define DYNAMIC_BUFFER_INIT { NULL, 0, 0 }
-#define NS_1_SECOND         1000000000      // 1 second, in nanoseconds
+#define NS_1_SECOND         1000000000U     // 1 second, in nanoseconds
 #define OUT_DATA_THRESHOLD  1024 * 10U      // 10KB, "a few screenfuls" of data.
 
 typedef struct {
@@ -344,13 +344,11 @@ static bool out_data_decide_throttle(size_t size)
   static uint64_t   started     = 0;  // Start time of the current throttle.
   static size_t     received    = 0;  // Bytes observed since last throttle.
   static size_t     visit       = 0;  // "Pulse" count of the current throttle.
-  static size_t     max_visits  = 0;
   static char       pulse_msg[] = { ' ', ' ', ' ', '\0' };
 
   if (!size) {
     bool previous_decision = (visit > 0);
     started = received = visit = 0;
-    max_visits = 20;
     return previous_decision;
   }
 
@@ -361,17 +359,9 @@ static bool out_data_decide_throttle(size_t size)
     return false;
   } else if (!visit) {
     started = os_hrtime();
-  } else if (visit >= max_visits && size < 256 && max_visits < 999) {
-    // Gobble up small chunks even if we maxed out. Avoids the case where the
-    // final displayed chunk is very tiny.
-    max_visits = visit + 1;
-  } else if (visit >= max_visits) {
+  } else if (visit % 20 == 0) {
     uint64_t since = os_hrtime() - started;
-    if (since < NS_1_SECOND) {
-      // Adjust max_visits based on the current relative performance.
-      // Each "pulse" period should last >=1 second so that it is perceptible.
-      max_visits = (2 * max_visits);
-    } else {
+    if (since > (3 * NS_1_SECOND)) {
       received = visit = 0;
       return false;
     }
@@ -379,7 +369,7 @@ static bool out_data_decide_throttle(size_t size)
 
   visit++;
   // Pulse "..." at the bottom of the screen.
-  size_t tick = (visit == max_visits)
+  size_t tick = (visit % 20 == 0)
                 ? 3  // Force all dots "..." on last visit.
                 : (visit % 4);
   pulse_msg[0] = (tick == 0) ? ' ' : '.';