diff --git a/mps/code/ld.c b/mps/code/ld.c
index 766bfe5d823..e74c90ec792 100644
--- a/mps/code/ld.c
+++ b/mps/code/ld.c
@@ -1,7 +1,7 @@
/* ld.c: LOCATION DEPENDENCY IMPLEMENTATION
*
* $Id$
- * Copyright (c) 2001 Ravenbrook Limited. See end of file for license.
+ * Copyright (c) 2001-2013 Ravenbrook Limited. See end of file for license.
*
* .def: A location dependency records the fact that the bit-patterns
* of some references will be used directly (most likely for
@@ -102,7 +102,7 @@ void LDAdd(mps_ld_t ld, Arena arena, Addr addr)
}
-/* LDIsStale -- check whether a dependency is stale
+/* LDIsStaleAny -- check whether any dependency is stale
*
* .stale.thread-safe: This function is thread safe. It will return a
* correct (but possibly conservative) answer regardless of the number
@@ -122,12 +122,10 @@ void LDAdd(mps_ld_t ld, Arena arena, Addr addr)
* .stale.old: Otherwise, if the dependency is older than the length
* of the history, check it against all movement that has ever occured.
*/
-Bool LDIsStale(mps_ld_t ld, Arena arena, Addr addr)
+Bool LDIsStaleAny(mps_ld_t ld, Arena arena)
{
RefSet rs;
- UNUSED(addr);
-
AVER(ld->_epoch <= arena->epoch);
/* AVERT(Arena, arena) -- .stale.thread-safe */
@@ -148,6 +146,19 @@ Bool LDIsStale(mps_ld_t ld, Arena arena, Addr addr)
}
+/* LDIsStaleAny -- check whether a particular dependency is stale
+ *
+ * .stale.conservative: In fact we just ignore the address and test if
+ * any dependency is stale. This is conservatively correct (no false
+ * negatives) but provides a hook for future improvement.
+ */
+Bool LDIsStale(mps_ld_t ld, Arena arena, Addr addr)
+{
+ UNUSED(addr);
+ return LDIsStaleAny(ld, arena);
+}
+
+
/* LDAge -- age the arena by adding a moved set
*
* This stores the fact that a set of references has changed in
@@ -212,7 +223,7 @@ void LDMerge(mps_ld_t ld, Arena arena, mps_ld_t from)
/* C. COPYRIGHT AND LICENSE
*
- * Copyright (C) 2001-2002 Ravenbrook Limited .
+ * Copyright (C) 2001-2013 Ravenbrook Limited .
* All rights reserved. This is an open source license. Contact
* Ravenbrook for commercial licensing options.
*
diff --git a/mps/code/mpm.h b/mps/code/mpm.h
index 6bdabf4cf62..37957a1bcc1 100644
--- a/mps/code/mpm.h
+++ b/mps/code/mpm.h
@@ -907,6 +907,7 @@ extern Res MutatorFaultContextScan(ScanState ss, MutatorFaultContext mfc);
extern void LDReset(mps_ld_t ld, Arena arena);
extern void LDAdd(mps_ld_t ld, Arena arena, Addr addr);
+extern Bool LDIsStaleAny(mps_ld_t ld, Arena arena);
extern Bool LDIsStale(mps_ld_t ld, Arena arena, Addr addr);
extern void LDAge(Arena arena, RefSet moved);
extern void LDMerge(mps_ld_t ld, Arena arena, mps_ld_t from);
diff --git a/mps/code/mps.h b/mps/code/mps.h
index 5bc46d4d1c9..dfcb894b8c3 100644
--- a/mps/code/mps.h
+++ b/mps/code/mps.h
@@ -661,6 +661,7 @@ extern void mps_ld_reset(mps_ld_t, mps_arena_t);
extern void mps_ld_add(mps_ld_t, mps_arena_t, mps_addr_t);
extern void mps_ld_merge(mps_ld_t, mps_arena_t, mps_ld_t);
extern mps_bool_t mps_ld_isstale(mps_ld_t, mps_arena_t, mps_addr_t);
+extern mps_bool_t mps_ld_isstale_any(mps_ld_t, mps_arena_t);
extern mps_word_t mps_collections(mps_arena_t);
diff --git a/mps/code/mpsi.c b/mps/code/mpsi.c
index dede080ea45..8aa7876a939 100644
--- a/mps/code/mpsi.c
+++ b/mps/code/mpsi.c
@@ -1480,6 +1480,15 @@ mps_bool_t mps_ld_isstale(mps_ld_t ld, mps_arena_t arena,
return (mps_bool_t)b;
}
+mps_bool_t mps_ld_isstale_any(mps_ld_t ld, mps_arena_t arena)
+{
+ Bool b;
+
+ b = LDIsStaleAny(ld, arena);
+
+ return (mps_bool_t)b;
+}
+
mps_res_t mps_fix(mps_ss_t mps_ss, mps_addr_t *ref_io)
{
mps_res_t res;
diff --git a/mps/code/mpsicv.c b/mps/code/mpsicv.c
index 4a3c95abe83..142498a4502 100644
--- a/mps/code/mpsicv.c
+++ b/mps/code/mpsicv.c
@@ -468,6 +468,7 @@ static void *test(void *arg, size_t s)
mps_ld_add(&ld, arena, obj);
if (mps_ld_isstale(&ld, arena, obj)) {
+ cdie(mps_ld_isstale_any(&ld, arena), "mps_ld_isstale_any");
mps_ld_reset(&ld, arena);
mps_ld_add(&ld, arena, obj);
}
diff --git a/mps/manual/source/glossary/l.rst b/mps/manual/source/glossary/l.rst
index 16bf68d24d9..5a143e55f69 100644
--- a/mps/manual/source/glossary/l.rst
+++ b/mps/manual/source/glossary/l.rst
@@ -239,8 +239,8 @@ Memory Management Glossary: L
:term:`references` (and not merely on the
identity of the :term:`block` to which the reference
refers), and provides a function
- (:c:func:`mps_ld_isstale`) to find out whether any of
- these references have been changed because a block has
+ (:c:func:`mps_ld_isstale`) to find out whether a
+ reference might have been changed because a block has
been :term:`moved `. See
:ref:`topic-location`.
diff --git a/mps/manual/source/guide/advanced.rst b/mps/manual/source/guide/advanced.rst
index 54680dcb484..04714d3b7d7 100644
--- a/mps/manual/source/guide/advanced.rst
+++ b/mps/manual/source/guide/advanced.rst
@@ -335,7 +335,7 @@ any of its keys.
If you look up a key in an address-based hash table and fail to find it
there, that might be because the table's dependency on the location of
the key is stale: that is, if the garbage collector moved the key. The
-function :c:func:`mps_ld_isstale` tells you if any of the blocks whose
+function :c:func:`mps_ld_isstale` tells you if a block whose
locations you depended upon since the last call to
:c:func:`mps_ld_reset` might have moved.
@@ -354,25 +354,17 @@ locations you depended upon since the last call to
return NULL;
}
-It's important to test :c:func:`mps_ld_isstale` only in case of failure.
-The function tells you whether *any* of the dependencies is stale, not
-whether a particular dependency is stale. So if ``key`` has not moved,
-but some other keys have moved, then if you tested
-:c:func:`mps_ld_isstale` first, it would return true and so you'd end up
-unnecessarily rehashing the whole table. (It's crucial, however, to
-actually test that ``key`` appears in the table, not just that some key
-with the same hash does.)
+It's important to test :c:func:`mps_ld_isstale` only in case of
+failure. The function may report a false positive (returning true
+despite the block not having moved). So if ``key`` has not moved, then
+if you tested :c:func:`mps_ld_isstale` first, it might return true and
+so you'd end up unnecessarily rehashing the whole table. (It's
+crucial, however, to actually test that ``key`` appears in the table,
+not just that some key with the same hash does.)
When a table is rehashed, call :c:func:`mps_ld_reset` to clear the
location dependency, and then :c:func:`mps_ld_add` for each key before it is added back to the table.
-.. note::
-
- Somewhat misleadingly, :c:func:`mps_ld_isstale` takes an address as
- its third argument. This address is not tested for staleness: it
- appears in the :term:`telemetry stream`, however, where it might be
- useful for debugging.
-
.. note::
After :c:func:`mps_ld_isstale` has returned true, and after
diff --git a/mps/manual/source/topic/location.rst b/mps/manual/source/topic/location.rst
index 5cf4507264d..d70d689415b 100644
--- a/mps/manual/source/topic/location.rst
+++ b/mps/manual/source/topic/location.rst
@@ -124,7 +124,7 @@ done in the function that hashes an address:
Testing dependencies for staleness
----------------------------------
-When the locations of blocks are used, first carry out the computation
+When the location of a block is used, first carry out the computation
in the normal way. For example, when looking up a key in an
address-based hash table, start by hashing the pointer and looking up
the corresponding index in the table.
@@ -135,26 +135,26 @@ required: the operation can proceed as usual.
But if the operation fails, you might be in one of two cases:
-1. the location of these blocks has not been depended on before (for
+1. the location of the block has not been depended on before (for
example, the key has never been added to the hash table);
-2. the location of these blocks has been depended on before (for
- example, the key was added to the hash table), but one or more of
- the blocks has moved and the dependency has become stale.
+2. the location of the block has been depended on before (for example,
+ the key was added to the hash table), but the block has moved and
+ the dependency has become stale.
At this point you should call :c:func:`mps_ld_isstale`. If it returns
-false, then you know that no blocks have moved, so you must be in case
+false, then you know that the block has not moved, so you must be in case
(1).
But if :c:func:`mps_ld_isstale` returns true, you could still be in
either case (1) or case (2). All :c:func:`mps_ld_isstale` tells you is
-that *some* blocks that have been depended on *might* have moved, not
-whether a *particular* block *has* moved. At this point you must:
+that the block *might* have moved, not whether the block *has* moved.
+At this point you must:
1. reset the location dependency;
-2. repeat the computation in some way that doesn't depend on the
- old locations of the blocks; and
+2. repeat the computation in some way that doesn't depend on the old
+ locations of all the blocks that were added to that dependency; and
3. re-add a dependency on each block.
@@ -285,8 +285,9 @@ Location dependency interface
.. c:function:: mps_bool_t mps_ld_isstale(mps_ld_t ld, mps_arena_t arena, mps_addr_t addr)
- Determine if any of the dependencies in a :term:`location
- dependency` are stale with respect to an :term:`arena`.
+ Determine if a dependency on the location of a block in a
+ :term:`location dependency` might be stale with respect to an
+ :term:`arena`.
``ld`` is the location dependency.
@@ -294,34 +295,58 @@ Location dependency interface
the same arena that was passed to all calls to
:c:func:`mps_ld_add` on ``ld``.
- ``addr`` is an address that may appear in :term:`telemetry
- ` events related to this call (it will *not* be
- tested for staleness).
+ ``addr`` is the address of the block that is to be tested for
+ staleness.
- The location dependency is examined to determine whether any of
- the dependencies encapsulated in it have been made stale with
- respect to ``arena``. If any of the dependencies encapsulated in
- the location dependency are stale (that is, the blocks whose
- location has been depended on have been moved by ``arena``) then
- :c:func:`mps_ld_isstale` will return true. If there have been no
- calls to :c:func:`mps_ld_add` on ``ld`` since the last call to
- :c:func:`mps_ld_reset`, then :c:func:`mps_ld_isstale` will return
- false. :c:func:`mps_ld_isstale` may return any value in other
- circumstances (but will strive to return false if the blocks
- encapsulated in the location dependency have not moved).
+ If there have been no calls to :c:func:`mps_ld_add` on ``ld``
+ since the last call to :c:func:`mps_ld_reset`, then return false.
+
+ If the block at ``addr`` was formerly added to the location
+ dependency ``ld`` and subsequently moved by ``arena``, then return
+ true.
+
+ Otherwise, :c:func:`mps_ld_isstale` may return either true or
+ false. (The function strives to return true in the case where
+ ``addr`` was added to the location dependency and subsquently
+ moved, and false otherwise, but cannot ensure this.)
.. note::
- :c:func:`mps_ld_isstale` may report a false positive
- (returning true despite none of the added addresses having
- being moved by the arena) but never a false negative
- (returning false when an added address has been moved).
+ :c:func:`mps_ld_isstale` may report a false positive: it may
+ return true in the case where ``addr`` was not added to the
+ location dependency, or in the case where it was added but not
+ moved. It never reports a false negative.
:c:func:`mps_ld_isstale` is thread-safe with respect to itself
and with respect to :c:func:`mps_ld_add`, but not with respect
to :c:func:`mps_ld_reset`.
+.. c:function:: mps_bool_t mps_ld_isstale_any(mps_ld_t ld, mps_arena_t arena)
+
+ Determine if a dependency on the location of *any* block in a
+ :term:`location dependency` might be stale with respect to an
+ :term:`arena`.
+
+ ``ld`` is the location dependency.
+
+ ``arena`` is the arena to test for staleness against. It must be
+ the same arena that was passed to all calls to
+ :c:func:`mps_ld_add` on ``ld``.
+
+ If there have been no calls to :c:func:`mps_ld_add` on ``ld``
+ since the last call to :c:func:`mps_ld_reset`, then return false.
+
+ If any block added to the location dependency ``ld`` has been
+ moved by ``arena``, then return true.
+
+ Otherwise, :c:func:`mps_ld_isstale_any` may return either true or
+ false. (The function strives to return true in the case where a
+ block was added to the location dependency and subsquently moved,
+ and false otherwise, but cannot ensure this.)
+
+
+
.. c:function:: void mps_ld_merge(mps_ld_t dest_ld, mps_arena_t arena, mps_ld_t src_ld)
Merge one :term:`location dependency` into another.