Re: Bug 688602 requires a fundamental change to the memory management.

"Leonardo" <[email protected]> Fri, 7 Apr 2006 00:49:36 +0400
Newsgroups gmane.comp.printing.ghostscript.patches
Message-ID <[email protected]>
This is a multi-part message in MIME format.

------=_NextPart_000_0016_01C659DD.24931020
Content-Type: text/plain; format=flowed; charset="koi8-r"; reply-type=original
Content-Transfer-Encoding: 7bit

This revision works with NO_INVISIBLE_LEVELS 1,
but there are problems. See log message in the attachment.

Leo.

------=_NextPart_000_0016_01C659DD.24931020
Content-Type: application/octet-stream;
	name="patch.txt "
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
	filename="patch.txt "

Not ready yet.
This patch works with NO_INVISIBLE_LEVELS 1,
but there are serious problems :

1. The garbager can't colledt ref arrays
   because they are referred from alloc_change_t::offset =3D=3D =
AC_OFFSET_ALLOCATED .
   One way to fix is to change gc_trace with a special handling of=20
   such elements.

2. Don't like changes to igcref.c .
   The old code used some smart invariants,
   which become invalid with=20
   alloc_change_t::offset =3D=3D AC_OFFSET_ALLOCATED
   elements.

3. The old code unites neighbour ref blocks,
   but this feature is lost with inserting
   alloc_change_t::offset =3D=3D AC_OFFSET_ALLOCATED
   elements. Need to allocate them before allocating=20
   a ref block.=20

4. Still not sure how the old code hadles empty ref arrays.
   It is not completely documented.
   We can see that they look as unreferenced objects
   during the relocation phase because=20
   they have no referred elements,
   so that o_back points to chead,
   but their relocation is done=20
   by the offset stored in the extra element
   which appears behind the end.
   Maybe that's all, but not sure.

   BTW, in most cases the old code
   unites them with a non-empty array,
   and we couldn't find a practical case
   when it is not united. We disabled
   the uniting condition to emulate
   and study this situation.
  =20

[Log message end]
Optimize setting the l_new marks during 'restore'.

DETAILS :

This is a preparation for fixing=20
Bug 688602 "Displaying file with GS-8.53 is much slower than with =
GS-8.51".
Currently the new code is disabled with the macro NO_INVISIBLE_LEVELS
defined in gxalloc.h .=20

When NO_INVISIBLE_LEVELS is zero,=20
an algorithmically equivalent change happens in isave.c,
and othewr modules are not affected.
In this case in isave.c :

1. Factored out a new function mark_allocated.
2. Added a new argument to save_set_new, save_set_new_changes
to know that it is called for an invisible level.

When NO_INVISIBLE_LEVELS is 1 :

1. Invisible save levels are not created.
2. Instead that, it creates a new kind of alloc_change_t element,=20
   which is marked with AC_OFFSET_ALLOCATED in the 'offset' field.
   Such element points to the beginning of a newly allocated 'ref' array
   (either packed or not).
3. save_set_new is expanded to process the new kind of alloc_change_t =
elements.
   With such elements it calls mark_allocated (see above),
   so that the effect is same as before the patch.
4. The scanning of allocated objects in save_set_new is removed
   due to (3).
5. save_set_new_changes is extended with computing the size of=20
   newly allocated 'ref' arrays, which are processed during the scan.
   It appears to be close to what save_set_new did while scanning
   allocated objects (the variable 'changed').
6. The new field gs_ref_memory_t::scan_limit stores a termination
   condition for scanning objects in save_set_new_changes.
   The value is stored when save_set_new_changes is called=20
   from alloc_save_state as it was for creatng an invisible save level.
   Rather the new code doesn't create an invisible level,=20
   the scan termination condition work as it were created.
7. save_set_new_changes uses the variable 'changed'
   for a proper setting of the termination condition.
   The condition is not exactly equivalent to the
   old one, because the old code also accounts=20
   a scanning of non-ref objects, but the new code
   doesn't bother with them. Therefore new termination conditions
   appear in some different points than invisible levels did.
   Therefore the behavior isn't algorithmically equivalent.
8. The new code always restore to a 'visible' level.
   In same time, doing a 'restore', some of l_new marks
   may be left unrestored due to the scan tremination condition.
   This effect may be considered as caching a reasonable
   number of recently changed objects by marking with l_new.
   Older changes loose the l_new mark and their further changes
   will be stored again in the 'changes' list.=20
   This causes an extra growth of the 'changes' list,
   but saves the processor time from redundant setting
   of multiple l_new marks when save-restore is executed
   multiple times after a big number of changes.

Thus NO_INVISIBLE_LEVELS 1 reconstructs the optimization,=20
which was lost while fixing the bug 688153 with the revision 5980.
Rather the behavior isn't strongly equivalent,
the main logic is mostly same.

Now we commit this patch with disabled new code.
We'll enable it separately after the regression testing
of the algorithmically equivalent part.

EXPECTED DIFFERENCES :

None.
[Log message end]
=20
=20
=20
=20
*** F:\SVN-GS\HEAD\gs\src\gsalloc.c	Wed Mar 15 15:04:22 2006
--- files\gs\src\gsalloc.c	Thu Apr  6 10:37:01 2006
***************
*** 86,89 ****
--- 86,92 ----
  ENUM_PTR3(0, gs_ref_memory_t, streams, names_array, changes);
  ENUM_PTR(3, gs_ref_memory_t, saved);
+ #if NO_INVISIBLE_LEVELS
+ ENUM_PTR(4, gs_ref_memory_t, scan_limit);
+ #endif
  ENUM_PTRS_END
  private RELOC_PTRS_WITH(ref_memory_reloc_ptrs, gs_ref_memory_t *mptr)
***************
*** 92,95 ****
--- 95,101 ----
      RELOC_PTR(gs_ref_memory_t, names_array);
      RELOC_PTR(gs_ref_memory_t, changes);
+ #if NO_INVISIBLE_LEVELS
+     RELOC_PTR(gs_ref_memory_t, scan_limit);
+ #endif
      /* Don't relocate the saved pointer now -- see igc.c for details. =
*/
      mptr->reloc_saved =3D RELOC_OBJ(mptr->saved);
***************
*** 323,326 ****
--- 329,335 ----
      mem->inherited =3D 0;
      mem->changes =3D 0;
+ #if NO_INVISIBLE_LEVELS
+     mem->scan_limit =3D 0;
+ #endif
      ialloc_reset_free(mem);
  }
=20
=20
=20
*** F:\SVN-GS\HEAD\gs\src\gxalloc.h	Wed Mar 15 15:04:25 2006
--- files\gs\src\gxalloc.h	Thu Apr  6 17:17:04 2006
***************
*** 27,30 ****
--- 27,32 ----
  #include "gxobj.h"
 =20
+ #define NO_INVISIBLE_LEVELS 1 /* old code =3D 0, new code =3D 1 */
+=20
  /* =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D Chunks =
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D */
 =20
***************
*** 381,384 ****
--- 383,389 ----
      int num_contexts;		/* # of contexts sharing this VM */
      struct alloc_change_s *changes;
+ #if NO_INVISIBLE_LEVELS
+     struct alloc_change_s *scan_limit;
+ #endif
      struct alloc_save_s *saved;
      long total_scanned;
***************
*** 397,401 ****
    gs_public_st_composite(st_ref_memory, gs_ref_memory_t,\
      "gs_ref_memory", ref_memory_enum_ptrs, ref_memory_reloc_ptrs)
! #define st_ref_memory_max_ptrs 4  /* streams, names_array, changes, =
saved */
 =20
  /* Define the procedures for the standard allocator. */
--- 402,406 ----
    gs_public_st_composite(st_ref_memory, gs_ref_memory_t,\
      "gs_ref_memory", ref_memory_enum_ptrs, ref_memory_reloc_ptrs)
! #define st_ref_memory_max_ptrs 5  /* streams, names_array, changes, =
scan_limit, saved */
 =20
  /* Define the procedures for the standard allocator. */
=20
=20
=20
*** F:\SVN-GS\HEAD\gs\src\ialloc.c	Wed Mar 15 15:04:20 2006
--- files\gs\src\ialloc.c	Thu Apr  6 23:53:33 2006
***************
*** 211,214 ****
--- 211,218 ----
  	    cl.cp->has_refs =3D true;
  	}
+ #if NO_INVISIBLE_LEVELS
+ 	if ((gs_memory_t *)mem !=3D mem->stable_memory)
+ 	    alloc_save_change_in(mem, (const ref *)-1, (ref_packed *)obj, =
"gs_alloc_ref_array");
+ #endif
      }
      make_array(parr, attrs | mem->space, num_refs, obj);
=20
=20
=20
*** F:\SVN-GS\HEAD\gs\src\igcref.c	Wed Mar 15 15:04:41 2006
--- files\gs\src\igcref.c	Thu Apr  6 15:53:21 2006
***************
*** 668,671 ****
--- 668,680 ----
  		r_clear_attrs(pref, l_mark);
  		src +=3D packed_per_ref;
+ #if NO_INVISIBLE_LEVELS
+ 		/* The last (the extra) ref may be marked from=20
+ 		alloc_change_t::offset=3D=3DAC_OFFSET_ALLOCATED,
+ 		if the extra ref ends an empty ref array. */
+ 		if (src >=3D end) {
+ 		    src -=3D packed_per_ref;
+ 		    break;
+ 		}
+ #endif
  	    }
      } else
***************
*** 692,697 ****
  		r_clear_attrs(&rtemp, l_mark);
  		ref_assign_inline((ref *) dest, &rtemp);
- 		dest +=3D packed_per_ref;
  		src +=3D packed_per_ref;
  	    } else {		/* check for end of block */
  		src +=3D packed_per_ref;
--- 701,713 ----
  		r_clear_attrs(&rtemp, l_mark);
  		ref_assign_inline((ref *) dest, &rtemp);
  		src +=3D packed_per_ref;
+ #if NO_INVISIBLE_LEVELS
+ 		/* The last (the extra) ref may be marked from=20
+ 		alloc_change_t::offset=3D=3DAC_OFFSET_ALLOCATED,
+ 		if the extra ref ends an empty ref array. */
+ 		if (src >=3D end)
+ 		    break;
+ #endif
+ 		dest +=3D packed_per_ref;
  	    } else {		/* check for end of block */
  		src +=3D packed_per_ref;
=20
=20
=20
*** F:\SVN-GS\HEAD\gs\src\isave.c	Wed Mar 15 15:04:49 2006
--- files\gs\src\isave.c	Thu Apr  6 13:46:12 2006
***************
*** 30,34 ****
  #include "gsutil.h"		/* gs_next_ids prototype */
 =20
-=20
  /* Structure descriptor */
  private_st_alloc_save();
--- 30,33 ----
***************
*** 166,169 ****
--- 165,171 ----
  #define AC_OFFSET_STATIC (-2)	/* static object */
  #define AC_OFFSET_REF (-1)	/* dynamic ref */
+ #if NO_INVISIBLE_LEVELS
+ #define AC_OFFSET_ALLOCATED (-3) /* a newly allocated ref array */
+ #endif
      short offset;		/* if >=3D 0, offset within struct */
  };
***************
*** 197,200 ****
--- 199,205 ----
  	    break;
  	case AC_OFFSET_REF:
+ #if NO_INVISIBLE_LEVELS
+ 	case AC_OFFSET_ALLOCATED:
+ #endif
  	    RELOC_REF_PTR_VAR(ptr->where);
  	    break;
***************
*** 250,255 ****
  private int  restore_resources(alloc_save_t *, gs_ref_memory_t *);
  private void restore_free(gs_ref_memory_t *);
! private long save_set_new(gs_ref_memory_t *, bool);
! private void save_set_new_changes(gs_ref_memory_t *, bool);
 =20
  /* Initialize the save/restore machinery. */
--- 255,260 ----
  private int  restore_resources(alloc_save_t *, gs_ref_memory_t *);
  private void restore_free(gs_ref_memory_t *);
! private long save_set_new(gs_ref_memory_t *, bool, bool);
! private void save_set_new_changes(gs_ref_memory_t *, bool, bool);
 =20
  /* Initialize the save/restore machinery. */
***************
*** 336,341 ****
      /* and ones in objects allocated since the last save. */
      if (lmem->save_level > 1) {
! 	long scanned =3D save_set_new(&lsave->state, false);
 =20
  	if ((lsave->state.total_scanned +=3D scanned) > max_repeated_scan) {
  	    /* Do a second, invisible save. */
--- 341,347 ----
      /* and ones in objects allocated since the last save. */
      if (lmem->save_level > 1) {
! 	long scanned =3D save_set_new(&lsave->state, false, true);
 =20
+ #if !NO_INVISIBLE_LEVELS
  	if ((lsave->state.total_scanned +=3D scanned) > max_repeated_scan) {
  	    /* Do a second, invisible save. */
***************
*** 365,368 ****
--- 371,377 ----
  	    }
  	}
+ #else
+ 	(void)scanned;
+ #endif
      }
      alloc_set_in_save(dmem);
***************
*** 439,442 ****
--- 448,455 ----
      register alloc_change_t *cp;
 =20
+ #if !NO_INVISIBLE_LEVELS
+     if (pcont =3D=3D (const ref *)-1)
+ 	return 0;
+ #endif
      if (mem->new_mask =3D=3D 0)
  	return 0;		/* no saving */
***************
*** 449,452 ****
--- 462,469 ----
      if (pcont =3D=3D NULL)
  	cp->offset =3D AC_OFFSET_STATIC;
+ #if NO_INVISIBLE_LEVELS
+     else if (pcont =3D=3D (const ref *)-1)
+ 	cp->offset =3D AC_OFFSET_ALLOCATED;
+ #endif
      else if (r_is_array(pcont) || r_has_type(pcont, t_dictionary))
  	cp->offset =3D AC_OFFSET_REF;
***************
*** 458,461 ****
--- 475,483 ----
  	gs_abort((const gs_memory_t *)mem);
      }
+ #if NO_INVISIBLE_LEVELS
+     if (cp->offset =3D=3D AC_OFFSET_ALLOCATED)
+ 	make_null(&cp->contents);
+     else
+ #endif
      if (r_is_packed(where))
  	*(ref_packed *)&cp->contents =3D *where;
***************
*** 694,698 ****
  	alloc_set_not_in_save(dmem);
      } else {			/* Set the l_new attribute in all slots that are now =
new. */
! 	save_set_new(mem, true);
      }
 =20
--- 716,720 ----
  	alloc_set_not_in_save(dmem);
      } else {			/* Set the l_new attribute in all slots that are now =
new. */
! 	save_set_new(mem, true, false);
      }
 =20
***************
*** 720,723 ****
--- 742,750 ----
  	    }
  #endif
+ #if NO_INVISIBLE_LEVELS
+ 	    if (cp->offset =3D=3D AC_OFFSET_ALLOCATED)
+ 		DO_NOTHING;
+ 	    else
+ #endif
  	    if (r_is_packed(&cp->contents))
  		*cp->where =3D *(ref_packed *) & cp->contents;
***************
*** 891,895 ****
  	    alloc_change_t *chp =3D mem->changes;
 =20
! 	    save_set_new(&sprev->state, true);
  	    /* Concatenate the changes chains. */
  	    if (chp =3D=3D 0)
--- 918,922 ----
  	    alloc_change_t *chp =3D mem->changes;
 =20
! 	    save_set_new(&sprev->state, true, false);
  	    /* Concatenate the changes chains. */
  	    if (chp =3D=3D 0)
***************
*** 904,908 ****
  	} else {
  	    forget_changes(mem);
! 	    save_set_new(mem, false);
  	    file_forget_save(mem);
  	    combine_space(mem);	/* combine memory */
--- 931,935 ----
  	} else {
  	    forget_changes(mem);
! 	    save_set_new(mem, false, false);
  	    file_forget_save(mem);
  	    combine_space(mem);	/* combine memory */
***************
*** 912,916 ****
  	    if (mem !=3D save->space_local && mem->saved !=3D 0) {
  		forget_changes(mem);
! 		save_set_new(mem, false);
  		file_forget_save(mem);
  		combine_space(mem);
--- 939,943 ----
  	    if (mem !=3D save->space_local && mem->saved !=3D 0) {
  		forget_changes(mem);
! 		save_set_new(mem, false, false);
  		file_forget_save(mem);
  		combine_space(mem);
***************
*** 1015,1018 ****
--- 1042,1050 ----
 =20
  	if_debug1('U', "[U]forgetting change 0x%lx\n", (ulong) chp);
+ #if NO_INVISIBLE_LEVELS
+ 	if (chp->offset =3D=3D AC_OFFSET_ALLOCATED)
+ 	    DO_NOTHING;
+ 	else
+ #endif
  	if (!r_is_packed(prp))
  	    r_clear_attrs((ref *) prp, l_new);
***************
*** 1045,1071 ****
  /* ------ Internal routines ------ */
 =20
! /* Set or reset the l_new attribute in every relevant slot. */
! /* This includes every slot on the current change chain, */
! /* and every (ref) slot allocated at this save level. */
! /* Return the number of bytes of data scanned. */
! private long
! save_set_new(gs_ref_memory_t * mem, bool to_new)
  {
!     long scanned =3D 0;
!=20
!     /* Handle the change chain. */
!     save_set_new_changes(mem, to_new);
!=20
!     /* Handle newly allocated ref objects. */
!     SCAN_MEM_CHUNKS(mem, cp) {
! 	if (cp->has_refs) {
! 	    bool has_refs =3D false;
!=20
! 	    SCAN_CHUNK_OBJECTS(cp)
! 		DO_ALL
! 		if_debug3('U', "[U]set_new scan(0x%lx(%u), %d)\n",
! 			  (ulong) pre, size, to_new);
! 	    if (pre->o_type =3D=3D &st_refs) {
! 		/* These are refs, scan them. */
  		ref_packed *prp =3D (ref_packed *) (pre + 1);
  		ref_packed *next =3D (ref_packed *) ((char *)prp + size);
--- 1077,1085 ----
  /* ------ Internal routines ------ */
 =20
! private inline uint
! mark_allocated(void *obj, bool to_new)
  {  =20
!     obj_header_t *pre =3D (obj_header_t *)obj - 1;
!     uint size =3D pre_obj_contents_size(pre);
      ref_packed *prp =3D (ref_packed *) (pre + 1);
      ref_packed *next =3D (ref_packed *) ((char *)prp + size);
***************
*** 1077,1084 ****
  #endif
 =20
- 		if_debug2('U', "[U]refs 0x%lx to 0x%lx\n",
- 			  (ulong) prp, (ulong) next);
- 		has_refs =3D true;
- 		scanned +=3D size;
  		/* We know that every block of refs ends with */
  		/* a full-size ref, so we only need the end check */
--- 1091,1094 ----
***************
*** 1106,1109 ****
--- 1116,1149 ----
  	}
  #undef RP_REF
+     return size;
+ }
+=20
+ /* Set or reset the l_new attribute in every relevant slot. */
+ /* This includes every slot on the current change chain, */
+ /* and every (ref) slot allocated at this save level. */
+ /* Return the number of bytes of data scanned. */
+ private long
+ save_set_new(gs_ref_memory_t * mem, bool to_new, bool set_limit)
+ {
+     long scanned =3D 0;
+=20
+     /* Handle the change chain. */
+     save_set_new_changes(mem, to_new, set_limit);
+=20
+ #if !NO_INVISIBLE_LEVELS
+     /* Handle newly allocated ref objects. */
+     SCAN_MEM_CHUNKS(mem, cp) {
+ 	if (cp->has_refs) {
+ 	    bool has_refs =3D false;
+=20
+ 	    SCAN_CHUNK_OBJECTS(cp)
+ 		DO_ALL
+ 		if_debug3('U', "[U]set_new scan(0x%lx(%u), %d)\n",
+ 			  (ulong) pre, size, to_new);
+ 	    if (pre->o_type =3D=3D &st_refs) {
+ 		/* These are refs, scan them. */
+ 		ref_packed *prp =3D (ref_packed *) (pre + 1);
+=20
+ 		scanned +=3D mark_allocated(prp, to_new);
  	    } else
  		scanned +=3D sizeof(obj_header_t);
***************
*** 1115,1118 ****
--- 1155,1159 ----
  	if_debug2('u', "[u]set_new (%s) scanned %ld\n",
  		  (to_new ? "restore" : "save"), scanned);
+ #endif
      return scanned;
  }
***************
*** 1120,1129 ****
  /* Set or reset the l_new attribute on the changes chain. */
  private void
! save_set_new_changes(gs_ref_memory_t * mem, bool to_new)
  {
      register alloc_change_t *chp =3D mem->changes;
      register uint new =3D (to_new ? l_new : 0);
 =20
      for (; chp; chp =3D chp->next) {
  	ref_packed *prp =3D chp->where;
 =20
--- 1161,1179 ----
  /* Set or reset the l_new attribute on the changes chain. */
  private void
! save_set_new_changes(gs_ref_memory_t * mem, bool to_new, bool =
set_limit)
  {
      register alloc_change_t *chp =3D mem->changes;
      register uint new =3D (to_new ? l_new : 0);
+ #if NO_INVISIBLE_LEVELS
+     long scanned =3D mem->total_scanned;
+ #endif
 =20
      for (; chp; chp =3D chp->next) {
+ #if NO_INVISIBLE_LEVELS
+ 	if (chp->offset =3D=3D AC_OFFSET_ALLOCATED) {
+ 	    scanned +=3D mark_allocated((void *)chp->where, to_new);
+ 	} else
+ #endif
+ 	{
  	    ref_packed *prp =3D chp->where;
 =20
***************
*** 1137,1139 ****
--- 1187,1203 ----
  	    }
  	}
+ #if NO_INVISIBLE_LEVELS
+ 	if (mem->scan_limit =3D=3D chp)
+ 	    break;
+ #endif
+     }
+ #if NO_INVISIBLE_LEVELS
+     if (set_limit) {
+ 	if (scanned >=3D max_repeated_scan) {
+ 	    mem->scan_limit =3D chp;
+ 	    mem->total_scanned =3D 0;
+ 	} else
+ 	    mem->total_scanned =3D scanned;
+     }
+ #endif
  }
=20

------=_NextPart_000_0016_01C659DD.24931020
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
gs-code-review mailing list
[email protected]
http://www.ghostscript.com/mailman/listinfo/gs-code-review

------=_NextPart_000_0016_01C659DD.24931020--