* Re: Why buffer_shared is a mistake
2012-12-08 13:58 ` Eli Zaretskii
@ 2012-12-10 7:29 ` Dmitry Antipov
2012-12-10 8:18 ` Eli Zaretskii
0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Antipov @ 2012-12-10 7:29 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 901 bytes --]
On 12/08/2012 05:58 PM, Eli Zaretskii wrote:
> buffer_shared is used only by xdisp.c, with the single exception of
> modify_overlay. How about changing modify_overlay to compute this
> value by itself?
This is definitely possible (by enhancing window_loop, for example).
But now I'm thinking about maintaining per-buffer window counter,
which value is independent from the redisplay actions. This opens
the way to a different optimizations (e.g. if we're modifying a buffer
which isn't shown in a window, we can avoid redisplay at all).
(My view of the whole thing is: Emacs global state is huge, and
redisplay is controlled by too much amount of global variables.
This is error-prone and makes some valuable things like threading
too complicated or even impossible. So it's desirable to deduce
redisplay actions from per-buffer/window/frame variables rather
than from the global ones.)
Dmitry
[-- Attachment #2: window_count.patch --]
[-- Type: text/plain, Size: 9914 bytes --]
=== modified file 'src/buffer.c'
--- src/buffer.c 2012-12-07 07:16:32 +0000
+++ src/buffer.c 2012-12-10 06:14:55 +0000
@@ -547,6 +547,8 @@
b->base_buffer = NULL;
/* No one shares the text with us now. */
b->indirections = 0;
+ /* No one shows us now. */
+ b->window_count = 0;
BUF_GAP_SIZE (b) = 20;
block_input ();
@@ -794,6 +796,8 @@
b->indirections = -1;
/* Notify base buffer that we share the text now. */
b->base_buffer->indirections++;
+ /* Always -1 for an indirect buffer. */
+ b->window_count = -1;
b->pt = b->base_buffer->pt;
b->begv = b->base_buffer->begv;
@@ -1929,10 +1933,16 @@
eassert (b->indirections == -1);
b->base_buffer->indirections--;
eassert (b->base_buffer->indirections >= 0);
+ /* Make sure that we wasn't confused. */
+ eassert (b->window_count == -1);
}
else
- /* No one shares our buffer text, can free it. */
- free_buffer_text (b);
+ {
+ /* No window showing us, right? */
+ eassert (b->window_count == 0);
+ /* No one shares our buffer text, can free it. */
+ free_buffer_text (b);
+ }
if (b->newline_cache)
{
@@ -3880,17 +3890,17 @@
BUF_COMPUTE_UNCHANGED (buf, start, end);
- /* If this is a buffer not in the selected window,
- we must do other windows. */
- if (buf != XBUFFER (XWINDOW (selected_window)->buffer))
- windows_or_buffers_changed = 1;
- /* If multiple windows show this buffer, we must do other windows. */
- else if (buffer_shared > 1)
- windows_or_buffers_changed = 1;
- /* If we modify an overlay at the end of the buffer, we cannot
- be sure that window end is still valid. */
- else if (end >= ZV && start <= ZV)
- windows_or_buffers_changed = 1;
+ /* If BUF is visible, consider updating the display if ... */
+ if (buffer_window_count (buf) > 0)
+ {
+ /* ... it's visible in other window than selected, */
+ if (buf != XBUFFER (XWINDOW (selected_window)->buffer))
+ windows_or_buffers_changed = 1;
+ /* ... or if we modify an overlay at the end of the buffer
+ and so we cannot be sure that window end is still valid. */
+ else if (end >= ZV && start <= ZV)
+ windows_or_buffers_changed = 1;
+ }
++BUF_OVERLAY_MODIFF (buf);
}
@@ -5125,6 +5135,9 @@
/* No one will share the text with these buffers, but let's play it safe. */
buffer_defaults.indirections = 0;
buffer_local_symbols.indirections = 0;
+ /* Likewise no one will display them. */
+ buffer_defaults.window_count = 0;
+ buffer_local_symbols.window_count = 0;
set_buffer_intervals (&buffer_defaults, NULL);
set_buffer_intervals (&buffer_local_symbols, NULL);
/* This is not strictly necessary, but let's make them initialized. */
=== modified file 'src/buffer.h'
--- src/buffer.h 2012-12-07 07:16:32 +0000
+++ src/buffer.h 2012-12-10 05:20:09 +0000
@@ -775,6 +775,10 @@
zero means that we're the only owner of this text. */
int indirections;
+ /* Number of windows showing this buffer. Always -1 for
+ an indirect buffer since it counts as its base buffer. */
+ int window_count;
+
/* A non-zero value in slot IDX means that per-buffer variable
with index IDX has a local value in this buffer. The index IDX
for a buffer-local variable is stored in that variable's slot
@@ -1173,7 +1177,15 @@
+ pos + BUF_BEG_ADDR (buf) - BEG_BYTE);
return STRING_CHAR (p);
}
-\f
+
+BUFFER_INLINE int
+buffer_window_count (struct buffer *b)
+{
+ if (b->base_buffer)
+ b = b->base_buffer;
+ return b->window_count;
+}
+
/* Overlays */
/* Return the marker that stands for where OV starts in the buffer. */
=== modified file 'src/insdel.c'
--- src/insdel.c 2012-12-03 14:13:06 +0000
+++ src/insdel.c 2012-12-10 05:39:05 +0000
@@ -1796,13 +1796,14 @@
ptrdiff_t *preserve_ptr)
{
struct buffer *base_buffer;
+ struct buffer *selected = XBUFFER (XWINDOW (selected_window)->buffer);
if (!NILP (BVAR (current_buffer, read_only)))
Fbarf_if_buffer_read_only ();
- /* Let redisplay consider other windows than selected_window
- if modifying another buffer. */
- if (XBUFFER (XWINDOW (selected_window)->buffer) != current_buffer)
+ /* If modifying another visible buffer, let redisplay
+ consider other windows than selected_window. */
+ if (selected != current_buffer && buffer_window_count (selected) > 0)
++windows_or_buffers_changed;
if (buffer_intervals (current_buffer))
=== modified file 'src/window.c'
--- src/window.c 2012-12-07 08:13:49 +0000
+++ src/window.c 2012-12-10 05:18:10 +0000
@@ -270,6 +270,35 @@
return w;
}
+/* Called when W's buffer slot is changed. ARG -1 means that W is about to
+ cease its buffer, and 1 means that W is about to set up the new one. */
+
+static void
+adjust_window_count (struct window *w, int arg)
+{
+ eassert (eabs (arg) == 1);
+ if (BUFFERP (w->buffer))
+ {
+ struct buffer *b = XBUFFER (w->buffer);
+
+ if (b->base_buffer)
+ b = b->base_buffer;
+ b->window_count += arg;
+ eassert (b->window_count >= 0);
+ }
+}
+
+/* Set W's buffer slot to VAL and recompute number
+ of windows showing VAL if it is a buffer. */
+
+void
+wset_buffer (struct window *w, Lisp_Object val)
+{
+ adjust_window_count (w, -1);
+ w->buffer = val;
+ adjust_window_count (w, 1);
+}
+
/* Build a frequently used 4-integer (X Y W H) list. */
static Lisp_Object
@@ -2967,11 +2996,12 @@
{
Lisp_Object tail, frame;
- /* A single call to window_loop won't do the job because it only
- considers frames on the current keyboard. So loop manually over
- frames, and handle each one. */
- FOR_EACH_FRAME (tail, frame)
- window_loop (REPLACE_BUFFER_IN_WINDOWS_SAFELY, buffer, 1, frame);
+ if (buffer_window_count (XBUFFER (buffer)) > 0)
+ /* A single call to window_loop won't do the job because it only
+ considers frames on the current keyboard. So loop manually over
+ frames, and handle each one. */
+ FOR_EACH_FRAME (tail, frame)
+ window_loop (REPLACE_BUFFER_IN_WINDOWS_SAFELY, buffer, 1, frame);
}
\f
/* If *ROWS or *COLS are too small a size for FRAME, set them to the
@@ -3308,7 +3338,7 @@
if (STRINGP (object))
object = Fget_buffer (object);
- if (BUFFERP (object) && BUFFER_LIVE_P (XBUFFER (object)))
+ if (BUFFERP (object) && buffer_window_count (XBUFFER (object)) > 0)
{
/* Walk all windows looking for buffer, and force update
of each of those windows. */
@@ -3391,6 +3421,8 @@
memcpy ((char *) p + sizeof (struct vectorlike_header),
(char *) o + sizeof (struct vectorlike_header),
word_size * VECSIZE (struct window));
+ /* P's buffer slot was changed. */
+ adjust_window_count (p, 1);
XSETWINDOW (parent, p);
p->sequence_number = ++sequence_number;
=== modified file 'src/window.h'
--- src/window.h 2012-11-02 10:34:26 +0000
+++ src/window.h 2012-12-10 05:16:57 +0000
@@ -351,11 +351,6 @@
/* Most code should use these functions to set Lisp fields in struct
window. */
WINDOW_INLINE void
-wset_buffer (struct window *w, Lisp_Object val)
-{
- w->buffer = val;
-}
-WINDOW_INLINE void
wset_frame (struct window *w, Lisp_Object val)
{
w->frame = val;
@@ -947,11 +942,6 @@
extern int cursor_type_changed;
-/* Number of windows displaying the selected buffer. Normally this is
- 1, but it can be more. */
-
-extern int buffer_shared;
-
/* If *ROWS or *COLS are too small a size for FRAME, set them to the
minimum allowable size. */
@@ -997,6 +987,8 @@
extern void temp_output_buffer_show (Lisp_Object);
extern void replace_buffer_in_windows (Lisp_Object);
extern void replace_buffer_in_windows_safely (Lisp_Object);
+/* This looks like a setter, but it is a bit special. */
+extern void wset_buffer (struct window *, Lisp_Object);
extern void init_window_once (void);
extern void init_window (void);
extern void syms_of_window (void);
=== modified file 'src/xdisp.c'
--- src/xdisp.c 2012-12-04 15:15:30 +0000
+++ src/xdisp.c 2012-12-10 07:05:16 +0000
@@ -515,11 +515,6 @@
static int overlay_arrow_seen;
-/* Number of windows showing the buffer of the selected
- window (or another buffer with the same base buffer). */
-
-int buffer_shared;
-
/* Vector containing glyphs for an ellipsis `...'. */
static Lisp_Object default_invis_vector[3];
@@ -10894,9 +10889,8 @@
static int
buffer_shared_and_changed (void)
{
- /* The variable buffer_shared is set in redisplay_window and
- indicates that we redisplay a buffer in different windows. */
- return (buffer_shared > 1 && UNCHANGED_MODIFIED < MODIFF);
+ return (buffer_window_count (current_buffer) > 1
+ && UNCHANGED_MODIFIED < MODIFF);
}
/* Nonzero if W doesn't reflect the actual state of current buffer due
@@ -13470,10 +13464,6 @@
FOR_EACH_FRAME (tail, frame)
XFRAME (frame)->updated_p = 0;
- /* Recompute # windows showing selected buffer. This will be
- incremented each time such a window is displayed. */
- buffer_shared = 0;
-
FOR_EACH_FRAME (tail, frame)
{
struct frame *f = XFRAME (frame);
@@ -15568,21 +15558,6 @@
if (mode_line_update_needed (w))
update_mode_line = 1;
- /* Count number of windows showing the selected buffer. An indirect
- buffer counts as its base buffer. */
- if (!just_this_one_p)
- {
- struct buffer *current_base, *window_base;
- current_base = current_buffer;
- window_base = XBUFFER (XWINDOW (selected_window)->buffer);
- if (current_base->base_buffer)
- current_base = current_base->base_buffer;
- if (window_base->base_buffer)
- window_base = window_base->base_buffer;
- if (current_base == window_base)
- buffer_shared++;
- }
-
/* Point refers normally to the selected window. For any other
window, set up appropriate value. */
if (!EQ (window, selected_window))
^ permalink raw reply [flat|nested] 4+ messages in thread