[m-rev.] Post commit review: Remove available-space field from region struct

Quan Phan quan.phan at cs.kuleuven.be
Thu Dec 20 03:31:25 AEDT 2007


Hi,

I will commit this because it only affect RBMM stuff.

Zoltan, can you take a look at the diff to make sure I do not introduce
any clear problems. It turns out that we do not need to move the
next_page field in the MR_RegionPage struct as the first field because the
pointer to last word of the data area can be computed by
last_page->MR_regionpage_space + MR_REGION_PAGE_SPACE_SIZE - 1.
Moreover, if we use the MR_regionpage_space pointer in such
calculations, the order of the fields in MR_RegionPage does not matter.
 
Regards,
Quan.

-------------- next part --------------
Estimated hours taken: 2
Branch: main

- Fix the bug that some variables for profiling which are declared only if
  MR_RBMM_PROFILING is defined but are used even when the flag is off.
- Collect more profiling information in RBMM.

runtime/mercury_region.h
runtime/mercury_region.c
	Implement the above things. 
-------------- next part --------------
cvs diff: Diffing analysis
cvs diff: Diffing bench
cvs diff: Diffing bench/progs
cvs diff: Diffing bench/progs/compress
cvs diff: Diffing bench/progs/icfp2000
cvs diff: Diffing bench/progs/icfp2001
cvs diff: Diffing bench/progs/nuc
cvs diff: Diffing bench/progs/ray
cvs diff: Diffing bench/progs/tree234
cvs diff: Diffing bindist
cvs diff: Diffing boehm_gc
cvs diff: Diffing boehm_gc/Mac_files
cvs diff: Diffing boehm_gc/cord
cvs diff: Diffing boehm_gc/cord/private
cvs diff: Diffing boehm_gc/doc
cvs diff: Diffing boehm_gc/include
cvs diff: Diffing boehm_gc/include/private
cvs diff: Diffing boehm_gc/libatomic_ops-1.2
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/doc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps/gcc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps/hpc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps/ibmc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps/icc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps/msftc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/src/atomic_ops/sysdeps/sunc
cvs diff: Diffing boehm_gc/libatomic_ops-1.2/tests
cvs diff: Diffing boehm_gc/tests
cvs diff: Diffing boehm_gc/windows-untested
cvs diff: Diffing boehm_gc/windows-untested/vc60
cvs diff: Diffing boehm_gc/windows-untested/vc70
cvs diff: Diffing boehm_gc/windows-untested/vc71
cvs diff: Diffing browser
cvs diff: Diffing browser/test
cvs diff: Diffing bytecode
cvs diff: Diffing bytecode/test
cvs diff: Diffing compiler
Index: compiler/options.m
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/compiler/options.m,v
retrieving revision 1.605
diff -u -r1.605 options.m
--- compiler/options.m	26 Nov 2007 05:13:20 -0000	1.605
+++ compiler/options.m	19 Dec 2007 16:06:24 -0000
@@ -1220,13 +1220,13 @@
     size_region_disj_fixed              -   int(3),
     size_region_commit_fixed            -   int(4),
     size_region_ite_protect             -   int(1),
-    size_region_ite_snapshot            -   int(4),
+    size_region_ite_snapshot            -   int(3),
     size_region_disj_protect            -   int(0),
                                         % size_region_disj_protect is no longer
                                         % used. It should be removed when the
                                         % runtime support for RBMM is more
                                         % stable.
-    size_region_disj_snapshot           -   int(4),
+    size_region_disj_snapshot           -   int(3),
     size_region_commit_entry            -   int(1),
     solver_type_auto_init               -   bool(no)
 ]).
cvs diff: Diffing compiler/notes
cvs diff: Diffing debian
cvs diff: Diffing debian/patches
cvs diff: Diffing deep
cvs diff: Diffing deep_profiler
cvs diff: Diffing deep_profiler/notes
cvs diff: Diffing detail
cvs diff: Diffing doc
cvs diff: Diffing extras
cvs diff: Diffing extras/aditi
cvs diff: Diffing extras/base64
cvs diff: Diffing extras/cgi
cvs diff: Diffing extras/complex_numbers
cvs diff: Diffing extras/complex_numbers/samples
cvs diff: Diffing extras/complex_numbers/tests
cvs diff: Diffing extras/concurrency
cvs diff: Diffing extras/concurrency/samples
cvs diff: Diffing extras/concurrency/samples/midi
cvs diff: Diffing extras/concurrency/tests
cvs diff: Diffing extras/curs
cvs diff: Diffing extras/curs/samples
cvs diff: Diffing extras/curses
cvs diff: Diffing extras/curses/sample
cvs diff: Diffing extras/dynamic_linking
cvs diff: Diffing extras/error
cvs diff: Diffing extras/exceptions
cvs diff: Diffing extras/fixed
cvs diff: Diffing extras/gator
cvs diff: Diffing extras/gator/generations
cvs diff: Diffing extras/gator/generations/1
cvs diff: Diffing extras/graphics
cvs diff: Diffing extras/graphics/easyx
cvs diff: Diffing extras/graphics/easyx/samples
cvs diff: Diffing extras/graphics/mercury_allegro
cvs diff: Diffing extras/graphics/mercury_allegro/examples
cvs diff: Diffing extras/graphics/mercury_allegro/samples
cvs diff: Diffing extras/graphics/mercury_allegro/samples/demo
cvs diff: Diffing extras/graphics/mercury_allegro/samples/mandel
cvs diff: Diffing extras/graphics/mercury_allegro/samples/pendulum2
cvs diff: Diffing extras/graphics/mercury_allegro/samples/speed
cvs diff: Diffing extras/graphics/mercury_glut
cvs diff: Diffing extras/graphics/mercury_opengl
cvs diff: Diffing extras/graphics/mercury_tcltk
cvs diff: Diffing extras/graphics/samples
cvs diff: Diffing extras/graphics/samples/calc
cvs diff: Diffing extras/graphics/samples/gears
cvs diff: Diffing extras/graphics/samples/maze
cvs diff: Diffing extras/graphics/samples/pent
cvs diff: Diffing extras/lazy_evaluation
cvs diff: Diffing extras/lazy_evaluation/examples
cvs diff: Diffing extras/lex
cvs diff: Diffing extras/lex/samples
cvs diff: Diffing extras/lex/tests
cvs diff: Diffing extras/log4m
cvs diff: Diffing extras/logged_output
cvs diff: Diffing extras/moose
cvs diff: Diffing extras/moose/samples
cvs diff: Diffing extras/moose/tests
cvs diff: Diffing extras/mopenssl
cvs diff: Diffing extras/morphine
cvs diff: Diffing extras/morphine/non-regression-tests
cvs diff: Diffing extras/morphine/scripts
cvs diff: Diffing extras/morphine/source
cvs diff: Diffing extras/net
cvs diff: Diffing extras/odbc
cvs diff: Diffing extras/opium_m
cvs diff: Diffing extras/opium_m/non-regression-tests
cvs diff: Diffing extras/opium_m/scripts
cvs diff: Diffing extras/opium_m/source
cvs diff: Diffing extras/posix
cvs diff: Diffing extras/posix/samples
cvs diff: Diffing extras/quickcheck
cvs diff: Diffing extras/quickcheck/tutes
cvs diff: Diffing extras/references
cvs diff: Diffing extras/references/samples
cvs diff: Diffing extras/references/tests
cvs diff: Diffing extras/solver_types
cvs diff: Diffing extras/solver_types/library
cvs diff: Diffing extras/stream
cvs diff: Diffing extras/stream/tests
cvs diff: Diffing extras/trailed_update
cvs diff: Diffing extras/trailed_update/samples
cvs diff: Diffing extras/trailed_update/tests
cvs diff: Diffing extras/windows_installer_generator
cvs diff: Diffing extras/windows_installer_generator/sample
cvs diff: Diffing extras/windows_installer_generator/sample/images
cvs diff: Diffing extras/xml
cvs diff: Diffing extras/xml/samples
cvs diff: Diffing extras/xml_stylesheets
cvs diff: Diffing java
cvs diff: Diffing java/library
cvs diff: Diffing java/runtime
cvs diff: Diffing library
cvs diff: Diffing lp_solve
cvs diff: Diffing lp_solve/lp_examples
cvs diff: Diffing mdbcomp
cvs diff: Diffing profiler
cvs diff: Diffing quickcheck
cvs diff: Diffing quickcheck/tutes
cvs diff: Diffing readline
cvs diff: Diffing readline/doc
cvs diff: Diffing readline/examples
cvs diff: Diffing readline/shlib
cvs diff: Diffing readline/support
cvs diff: Diffing robdd
cvs diff: Diffing runtime
Index: runtime/mercury_region.c
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_region.c,v
retrieving revision 1.5
diff -u -r1.5 mercury_region.c
--- runtime/mercury_region.c	7 Dec 2007 10:36:34 -0000	1.5
+++ runtime/mercury_region.c	19 Dec 2007 16:06:25 -0000
@@ -149,15 +149,14 @@
     page = MR_region_get_free_page();
 
     /*
-    ** In the first page we will store region information, which occupies
-    ** word_sizeof(MR_Region) words from the start of the first page.
+    ** In the first page, we will store the region header, which occupies
+    ** word_sizeof(MR_Region) words from the start of the data area of the
+    ** page.
     */
     region = (MR_Region *) (page->MR_regionpage_space);
     region->MR_region_next_available_word = (MR_Word *)
         (page->MR_regionpage_space + word_sizeof(MR_Region));
     region->MR_region_last_page = page;
-    region->MR_region_available_space =
-        MR_REGION_PAGE_SPACE_SIZE - word_sizeof(MR_Region);
     region->MR_region_removal_counter = 1;
     region->MR_region_sequence_number = MR_region_sequence_number++;
     region->MR_region_logical_removed = 0;
@@ -279,12 +278,19 @@
 /*
 ** This method is to be called at the start of the then part of an ite with
 ** semidet condition (most of the times).
-** At that point we will only check if the region is disj-protected or not.
+** XXX At that point we will only check if the region is disj-protected or not.
+** At that point we do not have to check whether this ite-protected region is 
+** disj-protected or not. It is because the region is protected for this ite
+** only if it has not been protected before the ite and from the start of
+** this ite to the point when this method is called, i.e., the condition, the
+** region cannot be disj-protected by any disjunctions because this condition
+** is semidet.
 */
 
 void
 MR_remove_undisjprotected_region_ite_then_semidet(MR_Region *region)
 {
+    /*
     MR_region_debug_try_remove_region(region);
     if ( !MR_region_is_disj_protected(region) ) {
         MR_region_destroy_region(region);
@@ -293,6 +299,9 @@
         region->MR_region_logical_removed = 1;
         MR_region_debug_logically_remove_region(region);
     }
+    */
+    MR_region_destroy_region(region);
+    MR_region_debug_destroy_region(region);
 }
 
 /*
@@ -343,9 +352,7 @@
             region->MR_region_destroy_at_commit = 1;
         }*/
 
-#if defined(MR_RBMM_DEBUG)
-        MR_region_logically_remove_region_msg(region);
-#endif
+        MR_region_debug_logically_remove_region(region);
     }
 }
 
@@ -354,14 +361,14 @@
 {
     MR_Word *allocated_cell;
 
-    if (region->MR_region_available_space < words) {
+    if (MR_region_available_space(region->MR_region_last_page,
+                region->MR_region_next_available_word) < words) {
         MR_region_extend_region(region);
     }
 
     allocated_cell = region->MR_region_next_available_word;
     /* Allocate in the increasing direction of address. */
     region->MR_region_next_available_word += words;
-    region->MR_region_available_space -= words;
 #if defined(MR_RBMM_PROFILING)
     MR_region_update_profiling_unit(&MR_rbmmp_words_used, words);
     ((MR_RegionPage *) region)->MR_regionpage_allocated_size += words;
@@ -374,12 +381,10 @@
 MR_region_extend_region(MR_Region *region)
 {
     MR_RegionPage   *page;
-
     page = MR_region_get_free_page();
     region->MR_region_last_page->MR_regionpage_next = page;
     region->MR_region_last_page = page;
     region->MR_region_next_available_word = page->MR_regionpage_space;
-    region->MR_region_available_space = MR_REGION_PAGE_SPACE_SIZE;
 }
 
 /* Destroy any marked regions allocated before the commit. */
@@ -484,7 +489,7 @@
 void
 MR_region_logically_remove_region_msg(MR_Region *region)
 {
-    printf("Logically removed region #%d.\n",
+    printf("Logically remove region #%d.\n",
         region->MR_region_sequence_number);
 }
 
@@ -1018,12 +1023,14 @@
         *new_pages = MR_region_get_number_of_pages(first_new_page,
              restoring_region->MR_region_last_page);
         *new_words = (*new_pages * MR_REGION_PAGE_SPACE_SIZE -
-             restoring_region->MR_region_available_space +
-             snapshot->MR_snapshot_saved_available_space);
+             MR_region_available_space(restoring_region->MR_region_last_page,
+                restoring_region->MR_region_next_available_word) +
+             MR_region_available_space(snapshot->MR_snapshot_saved_last_page,
+                 snapshot->MR_snapshot_saved_next_available_word));
     } else {
         *new_pages = 0;
-        *new_words = snapshot->MR_snapshot_saved_available_space -
-            restoring_region->MR_region_available_space;
+        *new_words = restoring_region->MR_region_next_available_word - 
+            snapshot->MR_snapshot_saved_next_available_word;
     }
 }
 
Index: runtime/mercury_region.h
===================================================================
RCS file: /home/mercury/mercury1/repository/mercury/runtime/mercury_region.h,v
retrieving revision 1.6
diff -u -r1.6 mercury_region.h
--- runtime/mercury_region.h	7 Dec 2007 10:36:34 -0000	1.6
+++ runtime/mercury_region.h	19 Dec 2007 16:06:26 -0000
@@ -48,21 +48,19 @@
 #define     MR_REGION_DISJ_FRAME_FIXED_SIZE                 3
 #define     MR_REGION_COMMIT_FRAME_FIXED_SIZE               4
 #define     MR_REGION_ITE_PROT_SIZE                         1
-#define     MR_REGION_ITE_SNAPSHOT_SIZE                     4
+#define     MR_REGION_ITE_SNAPSHOT_SIZE                     3
 /*
-** The MR_REGION_DISJ_PROT_SIZE is no longer used. It should be removed when
-** the runtime support for RBMM becomes more stable. When removing it
+** XXX The MR_REGION_DISJ_PROT_SIZE is no longer used. It should be removed
+** when the runtime support for RBMM becomes more stable. When removing it
 ** remember to make the compiler/options.m consistent with the removal here.
 */
 #define     MR_REGION_DISJ_PROT_SIZE                        0
-#define     MR_REGION_DISJ_SNAPSHOT_SIZE                    4
+#define     MR_REGION_DISJ_SNAPSHOT_SIZE                    3
 #define     MR_REGION_COMMIT_SAVE_SIZE                      1
 
 /*
 ** A region page contains an array (of MR_Word) to store program data and
-** a pointer to the next to form a single-linked-list.  Note: the order of
-** fields in this struct is important. We make use of the order for several
-** computations (such as the address of a region).
+** a pointer to the next to form a single-linked-list.
 */
 struct MR_RegionPage_Struct {
     /* The space to store program data. */
@@ -101,14 +99,7 @@
     MR_Word                             *MR_region_next_available_word;
 
     /*
-    ** The current number of words (in the last page) available for allocation
-    ** into the region. When an allocation requires more words than what is
-    ** available the region is extended by adding a new page.
-    */
-    MR_Word                             MR_region_available_space;
-
-    /*
-    ** Currently not used. To be used for implementing conditional removals
+    ** XXX Currently not used. To be used for implementing conditional removals
     ** of regions, i.e., the region is only actually removed when the counter
     ** equals to one.
     */
@@ -118,6 +109,7 @@
     MR_Word                             MR_region_sequence_number;
 
     /* If the region has been removed in forward execution. */
+    /* XXX Currently it is not used for anything, we just maintain it */
     MR_Word                             MR_region_logical_removed;
 
     /*
@@ -178,7 +170,6 @@
     MR_Region           *MR_snapshot_region;
     MR_RegionPage       *MR_snapshot_saved_last_page;
     MR_Word             *MR_snapshot_saved_next_available_word;
-    MR_Word             MR_snapshot_saved_available_space;
 };
 
 /* Protection information in an ite frame. */
@@ -246,6 +237,10 @@
                     MR_region_alloc((MR_Region *) (region), (num)));        \
             } while (0)
 
+#define     MR_region_available_space(region_last_page, next_available_word)\
+            ((region_last_page->MR_regionpage_space) +                      \
+             MR_REGION_PAGE_SPACE_SIZE - next_available_word)               \
+
 extern  int             MR_region_is_disj_protected(MR_Region *region);
 
 /*---------------------------------------------------------------------------*/
@@ -733,8 +728,7 @@
                     protected_region = ite_prot->MR_ite_prot_region;        \
                     MR_region_debug_ite_unprotect(protected_region);        \
                     /* Try to protect the region by an outer condition. */  \
-                    protected_region->MR_region_ite_protected =             \
-                        top_ite_frame->MR_riff_previous_ite_frame;          \
+                    protected_region->MR_region_ite_protected = NULL;       \
                 }                                                           \
                 MR_region_debug_end("ite_unprotect");                       \
             } while (0)
@@ -811,8 +805,6 @@
                     (region)->MR_region_last_page;                          \
                 snapshot->MR_snapshot_saved_next_available_word =           \
                     (region)->MR_region_next_available_word;                \
-                snapshot->MR_snapshot_saved_available_space =               \
-                    (region)->MR_region_available_space;                    \
             } while (0)
 
                 
@@ -851,8 +843,6 @@
                     } /* else no new page added. */                         \
                     restoring_region->MR_region_next_available_word =       \
                         snapshot->MR_snapshot_saved_next_available_word;    \
-                    restoring_region->MR_region_available_space =           \
-                        snapshot->MR_snapshot_saved_available_space;        \
                 }                                                           \
             } while(0)
 
cvs diff: Diffing runtime/GETOPT
cvs diff: Diffing runtime/machdeps
cvs diff: Diffing samples
cvs diff: Diffing samples/c_interface
cvs diff: Diffing samples/c_interface/c_calls_mercury
cvs diff: Diffing samples/c_interface/cplusplus_calls_mercury
cvs diff: Diffing samples/c_interface/mercury_calls_c
cvs diff: Diffing samples/c_interface/mercury_calls_cplusplus
cvs diff: Diffing samples/c_interface/mercury_calls_fortran
cvs diff: Diffing samples/c_interface/simpler_c_calls_mercury
cvs diff: Diffing samples/c_interface/simpler_cplusplus_calls_mercury
cvs diff: Diffing samples/c_interface/standalone_c
cvs diff: Diffing samples/diff
cvs diff: Diffing samples/muz
cvs diff: Diffing samples/rot13
cvs diff: Diffing samples/solutions
cvs diff: Diffing samples/solver_types
cvs diff: Diffing samples/tests
cvs diff: Diffing samples/tests/c_interface
cvs diff: Diffing samples/tests/c_interface/c_calls_mercury
cvs diff: Diffing samples/tests/c_interface/cplusplus_calls_mercury
cvs diff: Diffing samples/tests/c_interface/mercury_calls_c
cvs diff: Diffing samples/tests/c_interface/mercury_calls_cplusplus
cvs diff: Diffing samples/tests/c_interface/mercury_calls_fortran
cvs diff: Diffing samples/tests/c_interface/simpler_c_calls_mercury
cvs diff: Diffing samples/tests/c_interface/simpler_cplusplus_calls_mercury
cvs diff: Diffing samples/tests/diff
cvs diff: Diffing samples/tests/muz
cvs diff: Diffing samples/tests/rot13
cvs diff: Diffing samples/tests/solutions
cvs diff: Diffing samples/tests/toplevel
cvs diff: Diffing scripts
cvs diff: Diffing slice
cvs diff: Diffing ssdb
cvs diff: Diffing tools
cvs diff: Diffing trace
cvs diff: Diffing trax
cvs diff: Diffing trial
cvs diff: Diffing util
cvs diff: Diffing vim
cvs diff: Diffing vim/after
cvs diff: Diffing vim/ftplugin
cvs diff: Diffing vim/syntax


More information about the reviews mailing list