On 05/11/2011 11:45 AM, Paolo Bonzini wrote:
Here is the patch, all the tests are green ;-)
Nice patch!
I have a few comments.
diff --git a/libgst/oop.c b/libgst/oop.c
index ad264d9..018ec95 100644
--- a/libgst/oop.c
+++ b/libgst/oop.c
@@ -66,6 +66,7 @@
/* Define this flag to turn on debugging code for OOP table management */
/* #define GC_DEBUGGING */
+#define GC_DEBUGGING
Spurious change.
@@ -701,7 +702,7 @@ _gst_make_oop_fixed (OOP oop)
{
gst_object newObj;
int size;
- if (oop->flags & F_FIXED)
+ if ((oop->flags & F_FIXED) || (oop->flags & F_LARGE))
return;
Can you try representing large objects with F_FIXED but not F_OLD? At
the same time, makeFixed would not cause objects to become old, which is
useful for objects passed to C.
Apparently, I even had some code to handle this in _gst_tenure_oop:
if (oop->flags & F_OLD)
return;
if (!(oop->flags & F_FIXED))
{
int size = SIZE_TO_BYTES (TO_INT(oop->object->objSize));
newObj = (gst_object) _gst_mem_alloc (_gst_mem.old, size);
if (!newObj)
abort ();
memcpy (newObj, oop->object, size);
_gst_mem.numOldOOPs++;
oop->object = newObj;
}
where the increment of numOldOOPs should be moved outside the if.
@@ -780,6 +781,38 @@ _gst_alloc_obj (size_t size,
}
gst_object
+_gst_alloc_large_obj (size_t size,
+ OOP *p_oop)
+{
+ gst_object p_instance;
+
+ size = ROUNDED_BYTES (size);
+
+ /* If the object is big enough, we put it directly in fixedspace. */
+ p_instance = (gst_object) _gst_mem_alloc (_gst_mem.fixed, size);
+ if COMMON (p_instance)
+ goto ok;
+
+ _gst_global_gc (size);
+ p_instance = (gst_object) _gst_mem_alloc (_gst_mem.fixed, size);
+ if COMMON (p_instance)
+ goto ok;
+
+ _gst_compact (0);
Compacting is not going to be helpful, for fixed objects.
@@ -1452,7 +1485,7 @@ _gst_sweep_oop (OOP oop)
_gst_make_oop_non_weak (oop);
/* Free unreachable oldspace objects. */
- if UNCOMMON (oop->flags & F_FIXED)
+ if UNCOMMON ((oop->flags & F_FIXED) || (oop->flags & F_LARGE))
{
_gst_mem.numOldOOPs--;
This makes stats wrong. With the idea above, the code would be like:
if ((oop->flags & F_LOADED) == 0)
{
if UNCOMMON (oop->flags & F_FIXED)
_gst_mem_free (_gst_mem.fixed, oop->object);
else if UNCOMMON (oop->flags & F_OLD)
_gst_mem_free (_gst_mem.old, oop->object);
}
if UNCOMMON (oop->flags & F_OLD)
{
_gst_mem.numOldOOPs--;
stats.reclaimedOldSpaceBytesSinceLastGlobalGC +=
SIZE_TO_BYTES (TO_INT (OOP_TO_OBJ (oop)->objSize));
}
It is a very good start though, thanks!
Paolo
Here is the new patch, I've removed like you've said the F_LARGE and
make the F_FIXED working with both new and old generation the spurious
change is removed ;-). I've included your idea just swap the two if
because the memory can be freed before the size is accessed.
Gwen
diff --git a/libgst/gstpriv.h b/libgst/gstpriv.h
index 68c34b9..1ad17e7 100644
--- a/libgst/gstpriv.h
+++ b/libgst/gstpriv.h
@@ -336,7 +336,7 @@ enum {
/* Set for objects that live in oldspace. */
F_OLD = 0x100U,
- /* Set together with F_OLD for objects that live in fixedspace. */
+ /* Set for objects that live in fixedspace. */
F_FIXED = 0x200U,
/* Set for untrusted classes, instances of untrusted classes,
diff --git a/libgst/oop.c b/libgst/oop.c
index ad264d9..fe8b4d2 100644
--- a/libgst/oop.c
+++ b/libgst/oop.c
@@ -720,8 +720,7 @@ _gst_make_oop_fixed (OOP oop)
oop->object = newObj;
}
- oop->flags &= ~(F_SPACES | F_POOLED);
- oop->flags |= F_OLD | F_FIXED;
+ oop->flags |= F_FIXED;
}
void
@@ -763,7 +762,7 @@ _gst_alloc_obj (size_t size,
newAllocPtr = _gst_mem.eden.allocPtr + BYTES_TO_SIZE (size);
if UNCOMMON (size >= _gst_mem.big_object_threshold)
- return _gst_alloc_old_obj (size, p_oop);
+ return _gst_alloc_large_obj (size, p_oop);
if UNCOMMON (newAllocPtr >= _gst_mem.eden.maxPtr)
{
@@ -780,6 +779,38 @@ _gst_alloc_obj (size_t size,
}
gst_object
+_gst_alloc_large_obj (size_t size,
+ OOP *p_oop)
+{
+ gst_object p_instance;
+
+ size = ROUNDED_BYTES (size);
+
+ /* If the object is big enough, we put it directly in fixedspace. */
+ p_instance = (gst_object) _gst_mem_alloc (_gst_mem.fixed, size);
+ if COMMON (p_instance)
+ goto ok;
+
+ _gst_global_gc (size);
+ p_instance = (gst_object) _gst_mem_alloc (_gst_mem.fixed, size);
+ if COMMON (p_instance)
+ goto ok;
+
+ _gst_compact (0);
+ p_instance = (gst_object) _gst_mem_alloc (_gst_mem.fixed, size);
+ if UNCOMMON (!p_instance)
+ {
+ _gst_errorf ("Cannot recover, exiting...");
+ exit (1);
+ }
+
+ok:
+ *p_oop = alloc_oop (p_instance, _gst_mem.active_flag | F_FIXED);
+ p_instance->objSize = FROM_INT (BYTES_TO_SIZE (size));
+ return p_instance;
+}
+
+gst_object
_gst_alloc_old_obj (size_t size,
OOP *p_oop)
{
@@ -1452,20 +1483,18 @@ _gst_sweep_oop (OOP oop)
_gst_make_oop_non_weak (oop);
/* Free unreachable oldspace objects. */
- if UNCOMMON (oop->flags & F_FIXED)
+ if UNCOMMON (oop->flags & F_OLD)
{
_gst_mem.numOldOOPs--;
stats.reclaimedOldSpaceBytesSinceLastGlobalGC +=
- SIZE_TO_BYTES (TO_INT (OOP_TO_OBJ (oop)->objSize));
- if ((oop->flags & F_LOADED) == 0)
- _gst_mem_free (_gst_mem.fixed, oop->object);
+ SIZE_TO_BYTES (TO_INT (OOP_TO_OBJ (oop)->objSize));
}
- else if UNCOMMON (oop->flags & F_OLD)
+
+ if ((oop->flags & F_LOADED) == 0)
{
- _gst_mem.numOldOOPs--;
- stats.reclaimedOldSpaceBytesSinceLastGlobalGC +=
- SIZE_TO_BYTES (TO_INT (OOP_TO_OBJ (oop)->objSize));
- if ((oop->flags & F_LOADED) == 0)
+ if UNCOMMON (oop->flags & F_FIXED)
+ _gst_mem_free (_gst_mem.fixed, oop->object);
+ else if UNCOMMON (oop->flags & F_OLD)
_gst_mem_free (_gst_mem.old, oop->object);
}
@@ -1617,7 +1646,9 @@ tenure_one_object ()
_gst_tenure_oop (oop);
queue_get (&_gst_mem.tenuring_queue, 1);
- queue_get (_gst_mem.active_half, TO_INT (oop->object->objSize));
+
+ if (!(oop->flags & F_FIXED))
+ queue_get (_gst_mem.active_half, TO_INT (oop->object->objSize));
}
void
@@ -2061,8 +2092,10 @@ _gst_copy_an_oop (OOP oop)
#endif
queue_put (&_gst_mem.tenuring_queue, &oop, 1);
- obj = oop->object = (gst_object)
- queue_put (_gst_mem.active_half, pData, TO_INT (obj->objSize));
+
+ if (!(oop->flags & F_FIXED))
+ obj = oop->object = (gst_object)
+ queue_put (_gst_mem.active_half, pData, TO_INT (obj->objSize));
oop->flags &= ~(F_SPACES | F_POOLED);
oop->flags |= _gst_mem.active_flag;
diff --git a/libgst/oop.h b/libgst/oop.h
index 6816b3f..4e99e76 100644
--- a/libgst/oop.h
+++ b/libgst/oop.h
@@ -409,6 +409,11 @@ extern gst_object _gst_alloc_obj (size_t size,
OOP *p_oop)
ATTRIBUTE_HIDDEN;
+/* The same, but for a large object */
+extern gst_object _gst_alloc_large_obj (size_t size,
+ OOP *p_oop)
+ ATTRIBUTE_HIDDEN;
+
/* The same, but for an oldspace object */
extern gst_object _gst_alloc_old_obj (size_t size,
OOP *p_oop)
diff --git a/snprintfv/snprintfv/filament.h b/snprintfv/snprintfv/filament.h
index 4a91eb6..8a7ce6c 100644
--- a/snprintfv/snprintfv/filament.h
+++ b/snprintfv/snprintfv/filament.h
@@ -1,4 +1,4 @@
-#line 1 "../../../snprintfv/snprintfv/filament.in"
+#line 1 "./filament.in"
/* -*- Mode: C -*- */
/* filament.h --- a bit like a string but different =)O|
@@ -118,7 +118,7 @@ extern char * fildelete (Filament *fil);
extern void _fil_extend (Filament *fil, size_t len, boolean copy);
-#line 61 "../../../snprintfv/snprintfv/filament.in"
+#line 61 "./filament.in"
/* Save the overhead of a function call in the great majority of cases. */
#define fil_maybe_extend(fil, len, copy) \
diff --git a/snprintfv/snprintfv/printf.h b/snprintfv/snprintfv/printf.h
index 49a2e9f..1437dd5 100644
--- a/snprintfv/snprintfv/printf.h
+++ b/snprintfv/snprintfv/printf.h
@@ -1,4 +1,4 @@
-#line 1 "../../../snprintfv/snprintfv/printf.in"
+#line 1 "./printf.in"
/* -*- Mode: C -*- */
/* printf.in --- printf clone for argv arrays
@@ -266,7 +266,7 @@ enum
} \
} SNV_STMT_END
-#line 269 "../../../snprintfv/snprintfv/printf.in"
+#line 269 "./printf.in"
/**
* printf_generic_info:
* @pinfo: the current state information for the format
@@ -302,7 +302,7 @@ extern int printf_generic_info (struct printf_info *const pinfo, size_t n, int *
extern int printf_generic (STREAM *stream, struct printf_info *const pinfo, union printf_arg const *args);
-#line 270 "../../../snprintfv/snprintfv/printf.in"
+#line 270 "./printf.in"
/**
* register_printf_function:
* @spec: the character which will trigger @func, cast to an unsigned int.
@@ -789,7 +789,7 @@ extern int snv_vasprintf (char **result, const char *format, va_list ap);
extern int snv_asprintfv (char **result, const char *format, snv_constpointer const args[]);
-#line 271 "../../../snprintfv/snprintfv/printf.in"
+#line 271 "./printf.in"
/* If you don't want to use snprintfv functions for *all* of your string
formatting API, then define COMPILING_SNPRINTFV_C and use the snv_
diff --git a/snprintfv/snprintfv/stream.h b/snprintfv/snprintfv/stream.h
index 496bd33..0bebce1 100644
--- a/snprintfv/snprintfv/stream.h
+++ b/snprintfv/snprintfv/stream.h
@@ -1,4 +1,4 @@
-#line 1 "../../../snprintfv/snprintfv/stream.in"
+#line 1 "./stream.in"
/* -*- Mode: C -*- */
/* stream.h --- customizable stream routines
@@ -180,7 +180,7 @@ extern int stream_puts (char *s, STREAM *stream);
extern int stream_get (STREAM *stream);
-#line 88 "../../../snprintfv/snprintfv/stream.in"
+#line 88 "./stream.in"
#ifdef __cplusplus
#if 0
/* This brace is so that emacs can still indent properly: */
_______________________________________________
help-smalltalk mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/help-smalltalk
