Re: Pseudovectors initialization

[Dan Nicolaescu <dann@gnu.org>]

Dmitry Antipov <dmantipov@yandex.ru> writes:

> Windows, frames, processes and terminals are created rarely and tends
> to have a long lifetime; so, IMHO, it's reasonable to pay a negligible
> performance penalty to simplify initialization, readability and maintenance.
> Any objections?
>
> Dmitry
>
> === modified file 'src/alloc.c'
> --- src/alloc.c	2012-06-26 05:00:30 +0000
> +++ src/alloc.c	2012-06-26 07:09:41 +0000
> @@ -3268,6 +3268,12 @@
>    for (i = 0; i < lisplen; ++i)
>      v->contents[i] = Qnil;
>  
> +  /* If the pseudovector is not a Lisp_Hash_Table and contains
> +     members beyond Lisp_Objects at the beginning, zero out them.  */
> +  if (tag != PVEC_HASH_TABLE && memlen > lisplen)
> +    memset ((char *) v + header_size + lisplen * word_size,
> +	    0, (memlen - lisplen) * word_size);
> +
>    XSETPVECTYPESIZE (v, tag, lisplen);
>    return v;
>  }
> @@ -3289,24 +3295,15 @@
>  struct terminal *
>  allocate_terminal (void)
>  {
> -  struct terminal *t = ALLOCATE_PSEUDOVECTOR (struct terminal,
> -					      next_terminal, PVEC_TERMINAL);
> -  /* Zero out the non-GC'd fields.  FIXME: This should be made unnecessary.  */
> -  memset (&t->next_terminal, 0,
> -	  (char*) (t + 1) - (char*) &t->next_terminal);
> -
> -  return t;
> +  return ALLOCATE_PSEUDOVECTOR (struct terminal,
> +				next_terminal, PVEC_TERMINAL);
>  }
>  
>  struct frame *
>  allocate_frame (void)
>  {
> -  struct frame *f = ALLOCATE_PSEUDOVECTOR (struct frame,
> -					   face_cache, PVEC_FRAME);
> -  /* Zero out the non-GC'd fields.  FIXME: This should be made unnecessary.  */
> -  memset (&f->face_cache, 0,
> -	  (char *) (f + 1) - (char *) &f->face_cache);
> -  return f;
> +  return ALLOCATE_PSEUDOVECTOR (struct frame,
> +				face_cache, PVEC_FRAME);
>  }
>  
>  
>
> === modified file 'src/frame.c'
> --- src/frame.c	2012-06-19 06:49:50 +0000
> +++ src/frame.c	2012-06-26 07:00:39 +0000
> @@ -267,63 +267,23 @@
>    f = allocate_frame ();
>    XSETFRAME (frame, f);
>  
> -  f->desired_matrix = 0;
> -  f->current_matrix = 0;
> -  f->desired_pool = 0;
> -  f->current_pool = 0;
> -  f->glyphs_initialized_p = 0;
> -  f->decode_mode_spec_buffer = 0;
> -  f->visible = 0;
> -  f->async_visible = 0;
> -  f->output_data.nothing = 0;
> -  f->iconified = 0;
> -  f->async_iconified = 0;
> +  /* Initialize Lisp data.  Note that allocate_frame initializes all
> +     Lisp data to nil, so do it only for slots which should not be nil.  */
> +  f->tool_bar_position = Qtop;
> +
> +  /* Initialize non-Lisp data.  Note that allocate_frame zeroes out all
> +     non-Lisp data, so do it only for slots which should not be zero.
> +     To avoid subtle bugs and for the sake of readability, it's better to
> +     initialize enum members explicitly even if their values are zero.  */
>    f->wants_modeline = 1;
> -  f->auto_raise = 0;
> -  f->auto_lower = 0;
> -  f->no_split = 0;
>    f->garbaged = 1;
>    f->has_minibuffer = mini_p;
> -  f->focus_frame = Qnil;

Is removing this, or removing anything that is set to Qnil safe?
It might be from case to case, but it's not obvious...


> -  f->explicit_name = 0;
> -  f->can_have_scroll_bars = 0;
>    f->vertical_scroll_bar_type = vertical_scroll_bar_none;
> -  f->param_alist = Qnil;
> -  f->scroll_bars = Qnil;
> -  f->condemned_scroll_bars = Qnil;
> -  f->face_alist = Qnil;
> -  f->face_cache = NULL;
> -  f->menu_bar_items = Qnil;
> -  f->menu_bar_vector = Qnil;
> -  f->menu_bar_items_used = 0;
> -  f->buffer_predicate = Qnil;
> -  f->buffer_list = Qnil;
> -  f->buried_buffer_list = Qnil;
> -  f->namebuf = 0;
> -  f->title = Qnil;
> -  f->menu_bar_window = Qnil;
> -  f->tool_bar_window = Qnil;
> -  f->tool_bar_items = Qnil;
> -  f->tool_bar_position = Qtop;
> -  f->desired_tool_bar_string = f->current_tool_bar_string = Qnil;
> -  f->n_tool_bar_items = 0;
> -  f->left_fringe_width = f->right_fringe_width = 0;
> -  f->fringe_cols = 0;
> -  f->menu_bar_lines = 0;
> -  f->tool_bar_lines = 0;
> -  f->scroll_bar_actual_width = 0;
> -  f->border_width = 0;
> -  f->internal_border_width = 0;
>    f->column_width = 1;  /* !FRAME_WINDOW_P value */
>    f->line_height = 1;  /* !FRAME_WINDOW_P value */
> -  f->x_pixels_diff = f->y_pixels_diff = 0;
>  #ifdef HAVE_WINDOW_SYSTEM
>    f->want_fullscreen = FULLSCREEN_NONE;
>  #endif
> -  f->size_hint_flags = 0;
> -  f->win_gravity = 0;
> -  f->font_driver_list = NULL;
> -  f->font_data_list = NULL;
>  
>    root_window = make_window ();
>    if (mini_p)
> @@ -399,8 +359,6 @@
>    ++window_select_count;
>    XSETFASTINT (XWINDOW (f->selected_window)->use_time, window_select_count);
>  
> -  f->default_face_done_p = 0;
> -
>    return f;
>  }
>  \f
>
> === modified file 'src/process.c'
> --- src/process.c	2012-06-24 20:34:48 +0000
> +++ src/process.c	2012-06-26 07:17:58 +0000
> @@ -625,35 +625,18 @@
>    printmax_t i;
>  
>    p = allocate_process ();
> +  /* Initialize Lisp data.  Note that allocate_process initializes all
> +     Lisp data to nil, so do it only for slots which should not be nil.  */
> +  p->status = Qrun;
> +  p->mark = Fmake_marker ();
>  
> +  /* Initialize non-Lisp data.  Note that allocate_process zeroes out all
> +     non-Lisp data, so do it only for slots which should not be zero.  */
>    p->infd = -1;
>    p->outfd = -1;
> -  p->tick = 0;
> -  p->update_tick = 0;
> -  p->pid = 0;
> -  p->pty_flag = 0;
> -  p->raw_status_new = 0;
> -  p->status = Qrun;
> -  p->mark = Fmake_marker ();
> -  p->kill_without_query = 0;
> -  p->write_queue = Qnil;
> -
> -#ifdef ADAPTIVE_READ_BUFFERING
> -  p->adaptive_read_buffering = 0;
> -  p->read_output_delay = 0;
> -  p->read_output_skip = 0;
> -#endif
>  
>  #ifdef HAVE_GNUTLS
>    p->gnutls_initstage = GNUTLS_STAGE_EMPTY;
> -  /* Default log level.  */
> -  p->gnutls_log_level = 0;
> -  /* GnuTLS handshakes attempted for this connection.  */
> -  p->gnutls_handshakes_tried = 0;
> -  p->gnutls_p = 0;
> -  p->gnutls_state = NULL;
> -  p->gnutls_x509_cred = NULL;
> -  p->gnutls_anon_cred = NULL;
>  #endif
>  
>    /* If name is already in use, modify it until it is unused.  */
>
> === modified file 'src/terminal.c'
> --- src/terminal.c	2012-01-19 07:21:25 +0000
> +++ src/terminal.c	2012-06-26 05:46:51 +0000
> @@ -225,7 +225,6 @@
>    struct terminal *terminal = allocate_terminal ();
>    Lisp_Object terminal_coding, keyboard_coding;
>  
> -  terminal->name = NULL;
>    terminal->next_terminal = terminal_list;
>    terminal_list = terminal;
>  
> @@ -255,9 +254,6 @@
>    setup_coding_system (keyboard_coding, terminal->keyboard_coding);
>    setup_coding_system (terminal_coding, terminal->terminal_coding);
>  
> -  terminal->param_alist = Qnil;
> -  terminal->charset_list = Qnil;
> -  terminal->Vselection_alist = Qnil;
>    return terminal;
>  }
>  
>
> === modified file 'src/window.c'
> --- src/window.c	2012-06-19 16:56:28 +0000
> +++ src/window.c	2012-06-26 07:17:40 +0000
> @@ -3242,13 +3242,12 @@
>  {
>    Lisp_Object parent;
>    register struct window *o, *p;
> -  int i;
>  
>    o = XWINDOW (window);
>    p = allocate_window ();
> -  for (i = 0; i < VECSIZE (struct window); ++i)
> -    ((struct Lisp_Vector *) p)->contents[i]
> -      = ((struct Lisp_Vector *) o)->contents[i];
> +  memcpy ((char *) p + sizeof (struct vectorlike_header), 
> +	  (char *) o + sizeof (struct vectorlike_header),
> +	  sizeof (Lisp_Object) * VECSIZE (struct window));
>    XSETWINDOW (parent, p);
>  
>    ++sequence_number;
> @@ -3277,10 +3276,8 @@
>    register struct window *w;
>  
>    w = allocate_window ();
> -  /* Initialize all Lisp data.  */
> -  w->frame = Qnil;
> -  w->mini = 0;
> -  w->next = w->prev = w->hchild = w->vchild = w->parent = Qnil;
> +  /* Initialize Lisp data.  Note that allocate_window initializes all
> +     Lisp data to nil, so do it only for slots which should not be nil.  */
>    XSETFASTINT (w->left_col, 0);
>    XSETFASTINT (w->top_line, 0);
>    XSETFASTINT (w->total_lines, 0);
> @@ -3289,47 +3286,24 @@
>    w->normal_cols = make_float (1.0);
>    XSETFASTINT (w->new_total, 0);
>    XSETFASTINT (w->new_normal, 0);
> -  w->buffer = Qnil;
>    w->start = Fmake_marker ();
>    w->pointm = Fmake_marker ();
> -  w->force_start = w->optional_new_start = 0;
>    XSETFASTINT (w->hscroll, 0);
>    XSETFASTINT (w->min_hscroll, 0);
>    XSETFASTINT (w->use_time, 0);
>    ++sequence_number;
>    XSETFASTINT (w->sequence_number, sequence_number);
> -  w->temslot = w->last_modified = w->last_overlay_modified = Qnil;
>    XSETFASTINT (w->last_point, 0);
> -  w->last_had_star = 0;
> -  w->vertical_scroll_bar = Qnil;
> -  w->left_margin_cols = w->right_margin_cols = Qnil;
> -  w->left_fringe_width = w->right_fringe_width = Qnil;
> -  w->fringes_outside_margins = Qnil;
> -  w->scroll_bar_width = Qnil;
>    w->vertical_scroll_bar_type = Qt;
>    XSETFASTINT (w->window_end_pos, 0);
>    XSETFASTINT (w->window_end_vpos, 0);
> -  w->window_end_valid = w->display_table = Qnil;
> -  w->update_mode_line = w->start_at_line_beg = 0;
> -  w->dedicated = Qnil;
> -  w->base_line_number = w->base_line_pos = w->region_showing = Qnil;
> -  w->column_number_displayed = w->redisplay_end_trigger = Qnil;
> -  w->combination_limit = w->window_parameters = Qnil;
> -  w->prev_buffers = w->next_buffers = Qnil;
> -  /* Initialize non-Lisp data.  */
> -  w->desired_matrix = w->current_matrix = 0;
> +
> +  /* Initialize non-Lisp data.  Note that allocate_window zeroes out all
> +     non-Lisp data, so do it only for slots which should not be zero.  */
>    w->nrows_scale_factor = w->ncols_scale_factor = 1;
> -  memset (&w->cursor, 0, sizeof (w->cursor));
> -  memset (&w->last_cursor, 0, sizeof (w->last_cursor));
> -  memset (&w->phys_cursor, 0, sizeof (w->phys_cursor));
>    w->phys_cursor_type = -1;
>    w->phys_cursor_width = -1;
> -  w->phys_cursor_on_p = 0;
> -  w->last_cursor_off_p = w->cursor_off_p = 0;
> -  w->must_be_updated_p = 0;
> -  w->pseudo_window_p = 0;
> -  w->frozen_window_start_p = 0;
> -  w->vscroll = 0;
> +
>    /* Reset window_list.  */
>    Vwindow_list = Qnil;
>    /* Return window.  */