From 38b272ffbbdaae276d636aec4ef84af407d16181 Mon Sep 17 00:00:00 2001 From: suzuki toshiya Date: Sun, 9 Jan 2011 22:49:07 +0900 Subject: [PATCH] [cache] Notice if a cache query induced the node list change. Some node comparators (comparing the cache node content and the properties specified by the query) can flush the cache node to prevent the cache inflation. The change may invalidate the pointers to the node obtained before the node comparison, so the change should be noticed to the caller. The problem caused by the cache node changing is reported by Harsha, see Savannah bug #31923. * src/cache/ftccache.h (FTC_Node_CompareFunc): Add new argument `FT_Bool* list_changed' to indicate the change of the cached nodes to the caller. (FTC_CACHE_LOOKUP_CMP): Watch the change of the cached nodes by `_list_changed'. (FTC_CACHE_TRYLOOP_END): Take new macro argument `_list_changed' and update it when FTC_Manager_FlushN() flushes any nodes. * src/cache/ftccback.h (ftc_snode_compare): Updated to fit with new FTC_Node_CompareFunc type. (ftc_gnode_compare): Ditto. * src/cache/ftcbasic.c: Include FT_INTERNAL_OBJECTS_H to use TRUE/FALSE macros. (ftc_basic_gnode_compare_faceid): New argument `FT_Bool* list_changed' to indicate the change of the cache nodes, anyway, it is always FALSE. * src/cache/ftccmap.c: Include FT_INTERNAL_OBJECTS_H to use TRUE/FALSE macros. (ftc_cmap_node_compare): New argument `FT_Bool* list_changed' to indicate the change of the cache nodes, anyway, it is always FALSE. (ftc_cmap_node_remove_faceid): Ditto. * src/cache/ftccache.c (FTC_Cache_NewNode): Pass a NULL pointer to FTC_CACHE_TRYLOOP_END(), because the result is not needed. (FTC_Cache_Lookup): Watch the change of the cache nodes by `list_changed'. (FTC_Cache_RemoveFaceID): Ditto. * src/cache/ftcglyph.c: Include FT_INTERNAL_OBJECTS_H to use TRUE/FALSE macros. (ftc_gnode_compare): New argument `FT_Bool* list_changed' to indicate the change of the cache nodes, anyway, it is always FALSE. (FTC_GNode_Compare): New argument `FT_Bool* list_changed' to be passed to ftc_gnode_compare(). * src/cache/ftcglyph.h (FTC_GNode_Compare): Ditto. * src/cache/ftcsbits.c (ftc_snode_compare): New argument `FT_Bool* list_changed' to indicate the change of the cache nodes, anyway. It is updated by FTC_CACHE_TRYLOOP(). (FTC_SNode_Compare): New argument `FT_Bool* list_changed' to be passed to ftc_snode_compare(). * src/cache/ftcsbits.h (FTC_SNode_Compare): Ditto. --- ChangeLog | 54 ++++++++++++++++++++++++++++++++++++++++++++ src/cache/ftcbasic.c | 6 ++++- src/cache/ftccache.c | 9 +++++--- src/cache/ftccache.h | 12 +++++++--- src/cache/ftccback.h | 6 +++-- src/cache/ftccmap.c | 11 +++++++-- src/cache/ftcglyph.c | 11 ++++++--- src/cache/ftcglyph.h | 3 ++- src/cache/ftcsbits.c | 12 ++++++---- src/cache/ftcsbits.h | 3 ++- 10 files changed, 107 insertions(+), 20 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0d8506d43..182476cf4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,57 @@ +2010-01-09 suzuki toshiya + + [cache] Notice if a cache query induced the node list change. + + Some node comparators (comparing the cache node content and + the properties specified by the query) can flush the cache + node to prevent the cache inflation. The change may + invalidate the pointers to the node obtained before the node + comparison, so the change should be noticed to the caller. + The problem caused by the cache node changing is reported by + Harsha, see Savannah bug #31923. + + * src/cache/ftccache.h (FTC_Node_CompareFunc): Add new + argument `FT_Bool* list_changed' to indicate the change of + the cached nodes to the caller. + (FTC_CACHE_LOOKUP_CMP): Watch the change of the cached nodes + by `_list_changed'. + (FTC_CACHE_TRYLOOP_END): Take new macro argument `_list_changed' + and update it when FTC_Manager_FlushN() flushes any nodes. + + * src/cache/ftccback.h (ftc_snode_compare): Updated to fit + with new FTC_Node_CompareFunc type. (ftc_gnode_compare): Ditto. + + * src/cache/ftcbasic.c: Include FT_INTERNAL_OBJECTS_H to + use TRUE/FALSE macros. (ftc_basic_gnode_compare_faceid): + New argument `FT_Bool* list_changed' to indicate the change + of the cache nodes, anyway, it is always FALSE. + + * src/cache/ftccmap.c: Include FT_INTERNAL_OBJECTS_H to + use TRUE/FALSE macros. (ftc_cmap_node_compare): + New argument `FT_Bool* list_changed' to indicate the change + of the cache nodes, anyway, it is always FALSE. + (ftc_cmap_node_remove_faceid): Ditto. + + * src/cache/ftccache.c (FTC_Cache_NewNode): Pass a NULL + pointer to FTC_CACHE_TRYLOOP_END(), because the result is + not needed. (FTC_Cache_Lookup): Watch the change of the cache + nodes by `list_changed'. (FTC_Cache_RemoveFaceID): Ditto. + + * src/cache/ftcglyph.c: Include FT_INTERNAL_OBJECTS_H to + use TRUE/FALSE macros. (ftc_gnode_compare): New argument + `FT_Bool* list_changed' to indicate the change of the cache + nodes, anyway, it is always FALSE. (FTC_GNode_Compare): + New argument `FT_Bool* list_changed' to be passed to + ftc_gnode_compare(). + * src/cache/ftcglyph.h (FTC_GNode_Compare): Ditto. + + * src/cache/ftcsbits.c (ftc_snode_compare): New argument + `FT_Bool* list_changed' to indicate the change of the cache + nodes, anyway. It is updated by FTC_CACHE_TRYLOOP(). + (FTC_SNode_Compare): New argument `FT_Bool* list_changed' + to be passed to ftc_snode_compare(). + * src/cache/ftcsbits.h (FTC_SNode_Compare): Ditto. + 2010-01-09 suzuki toshiya [cache] Fit FTC_GNode_Compare() to FTC_Node_CompareFunc. diff --git a/src/cache/ftcbasic.c b/src/cache/ftcbasic.c index 609ff78e5..48cfab0fe 100644 --- a/src/cache/ftcbasic.c +++ b/src/cache/ftcbasic.c @@ -17,6 +17,7 @@ #include +#include FT_INTERNAL_OBJECTS_H #include FT_INTERNAL_DEBUG_H #include FT_CACHE_H #include "ftcglyph.h" @@ -237,7 +238,8 @@ FT_CALLBACK_DEF( FT_Bool ) ftc_basic_gnode_compare_faceid( FTC_Node ftcgnode, FT_Pointer ftcface_id, - FTC_Cache cache ) + FTC_Cache cache, + FT_Bool* list_changed ) { FTC_GNode gnode = (FTC_GNode)ftcgnode; FTC_FaceID face_id = (FTC_FaceID)ftcface_id; @@ -245,6 +247,8 @@ FT_Bool result; + if ( list_changed ) + *list_changed = FALSE; result = FT_BOOL( family->attrs.scaler.face_id == face_id ); if ( result ) { diff --git a/src/cache/ftccache.c b/src/cache/ftccache.c index f28176e1e..9a5256017 100644 --- a/src/cache/ftccache.c +++ b/src/cache/ftccache.c @@ -461,7 +461,7 @@ { error = cache->clazz.node_new( &node, query, cache ); } - FTC_CACHE_TRYLOOP_END(); + FTC_CACHE_TRYLOOP_END( NULL ); if ( error ) node = NULL; @@ -490,6 +490,7 @@ FTC_Node* pnode; FTC_Node node; FT_Error error = FTC_Err_Ok; + FT_Bool list_changed = FALSE; FTC_Node_CompareFunc compare = cache->clazz.node_compare; @@ -504,7 +505,8 @@ if ( node == NULL ) goto NewNode; - if ( node->hash == hash && compare( node, query, cache ) ) + if ( node->hash == hash && + compare( node, query, cache, &list_changed ) ) break; pnode = &node->link; @@ -554,12 +556,13 @@ for ( ;; ) { FTC_Node node = *pnode; + FT_Bool list_changed = FALSE; if ( node == NULL ) break; - if ( cache->clazz.node_remove_faceid( node, face_id, cache ) ) + if ( cache->clazz.node_remove_faceid( node, face_id, cache, &list_changed ) ) { *pnode = node->link; node->link = frees; diff --git a/src/cache/ftccache.h b/src/cache/ftccache.h index 281ff391a..b6b40a01f 100644 --- a/src/cache/ftccache.h +++ b/src/cache/ftccache.h @@ -115,7 +115,8 @@ FT_BEGIN_HEADER typedef FT_Bool (*FTC_Node_CompareFunc)( FTC_Node node, FT_Pointer key, - FTC_Cache cache ); + FTC_Cache cache, + FT_Bool* list_changed ); typedef void @@ -218,6 +219,7 @@ FT_BEGIN_HEADER FTC_Cache _cache = FTC_CACHE(cache); \ FT_PtrDist _hash = (FT_PtrDist)(hash); \ FTC_Node_CompareFunc _nodcomp = (FTC_Node_CompareFunc)(nodecmp); \ + FT_Bool _list_changed = FALSE; \ \ \ error = FTC_Err_Ok; \ @@ -230,7 +232,8 @@ FT_BEGIN_HEADER if ( _node == NULL ) \ goto _NewNode; \ \ - if ( _node->hash == _hash && _nodcomp( _node, query, _cache ) ) \ + if ( _node->hash == _hash && \ + _nodcomp( _node, query, _cache, &_list_changed ) ) \ break; \ \ _pnode = &_node->link; \ @@ -299,11 +302,14 @@ FT_BEGIN_HEADER FT_UInt _try_done; -#define FTC_CACHE_TRYLOOP_END() \ +#define FTC_CACHE_TRYLOOP_END( list_changed ) \ if ( !error || error != FTC_Err_Out_Of_Memory ) \ break; \ \ _try_done = FTC_Manager_FlushN( _try_manager, _try_count ); \ + if ( _try_done > 0 && ( list_changed ) ) \ + *(FT_Bool*)( list_changed ) = TRUE; \ + \ if ( _try_done == 0 ) \ break; \ \ diff --git a/src/cache/ftccback.h b/src/cache/ftccback.h index 4d0818db2..4f5a32629 100644 --- a/src/cache/ftccback.h +++ b/src/cache/ftccback.h @@ -57,13 +57,15 @@ FT_LOCAL( FT_Bool ) ftc_snode_compare( FTC_Node snode, FT_Pointer gquery, - FTC_Cache cache ); + FTC_Cache cache, + FT_Bool* list_changed ); FT_LOCAL( FT_Bool ) ftc_gnode_compare( FTC_Node gnode, FT_Pointer gquery, - FTC_Cache cache ); + FTC_Cache cache, + FT_Bool* list_changed ); FT_LOCAL( FT_Error ) diff --git a/src/cache/ftccmap.c b/src/cache/ftccmap.c index 15060baf7..b0e85d1a9 100644 --- a/src/cache/ftccmap.c +++ b/src/cache/ftccmap.c @@ -22,6 +22,7 @@ #include FT_CACHE_H #include "ftcmanag.h" #include FT_INTERNAL_MEMORY_H +#include FT_INTERNAL_OBJECTS_H #include FT_INTERNAL_DEBUG_H #include "ftccback.h" @@ -190,13 +191,16 @@ FT_CALLBACK_DEF( FT_Bool ) ftc_cmap_node_compare( FTC_Node ftcnode, FT_Pointer ftcquery, - FTC_Cache cache ) + FTC_Cache cache, + FT_Bool* list_changed ) { FTC_CMapNode node = (FTC_CMapNode)ftcnode; FTC_CMapQuery query = (FTC_CMapQuery)ftcquery; FT_UNUSED( cache ); + if ( list_changed ) + *list_changed = FALSE; if ( node->face_id == query->face_id && node->cmap_index == query->cmap_index ) { @@ -213,12 +217,15 @@ FT_CALLBACK_DEF( FT_Bool ) ftc_cmap_node_remove_faceid( FTC_Node ftcnode, FT_Pointer ftcface_id, - FTC_Cache cache ) + FTC_Cache cache, + FT_Bool* list_changed ) { FTC_CMapNode node = (FTC_CMapNode)ftcnode; FTC_FaceID face_id = (FTC_FaceID)ftcface_id; FT_UNUSED( cache ); + if ( list_changed ) + *list_changed = FALSE; return FT_BOOL( node->face_id == face_id ); } diff --git a/src/cache/ftcglyph.c b/src/cache/ftcglyph.c index 52a79335b..ff041e89e 100644 --- a/src/cache/ftcglyph.c +++ b/src/cache/ftcglyph.c @@ -17,6 +17,7 @@ #include +#include FT_INTERNAL_OBJECTS_H #include FT_CACHE_H #include "ftcglyph.h" #include FT_ERRORS_H @@ -64,13 +65,16 @@ FT_LOCAL_DEF( FT_Bool ) ftc_gnode_compare( FTC_Node ftcgnode, FT_Pointer ftcgquery, - FTC_Cache cache ) + FTC_Cache cache, + FT_Bool* list_changed ) { FTC_GNode gnode = (FTC_GNode)ftcgnode; FTC_GQuery gquery = (FTC_GQuery)ftcgquery; FT_UNUSED( cache ); + if ( list_changed ) + *list_changed = FALSE; return FT_BOOL( gnode->family == gquery->family && gnode->gindex == gquery->gindex ); } @@ -81,9 +85,10 @@ FT_LOCAL_DEF( FT_Bool ) FTC_GNode_Compare( FTC_GNode gnode, FTC_GQuery gquery, - FTC_Cache cache ) + FTC_Cache cache, + FT_Bool* list_changed ) { - return ftc_gnode_compare( FTC_NODE( gnode ), gquery, cache ); + return ftc_gnode_compare( FTC_NODE( gnode ), gquery, cache, list_changed ); } #endif diff --git a/src/cache/ftcglyph.h b/src/cache/ftcglyph.h index 36567438c..d15ca3cd8 100644 --- a/src/cache/ftcglyph.h +++ b/src/cache/ftcglyph.h @@ -188,7 +188,8 @@ FT_BEGIN_HEADER FT_LOCAL( FT_Bool ) FTC_GNode_Compare( FTC_GNode gnode, FTC_GQuery gquery, - FTC_Cache cache ); + FTC_Cache cache, + FT_Bool* list_changed ); #endif diff --git a/src/cache/ftcsbits.c b/src/cache/ftcsbits.c index 52972a6f3..d4db99416 100644 --- a/src/cache/ftcsbits.c +++ b/src/cache/ftcsbits.c @@ -324,7 +324,8 @@ FT_LOCAL_DEF( FT_Bool ) ftc_snode_compare( FTC_Node ftcsnode, FT_Pointer ftcgquery, - FTC_Cache cache ) + FTC_Cache cache, + FT_Bool* list_changed ) { FTC_SNode snode = (FTC_SNode)ftcsnode; FTC_GQuery gquery = (FTC_GQuery)ftcgquery; @@ -333,6 +334,8 @@ FT_Bool result; + if (list_changed) + *list_changed = FALSE; result = FT_BOOL( gnode->family == gquery->family && (FT_UInt)( gindex - gnode->gindex ) < snode->count ); if ( result ) @@ -386,7 +389,7 @@ { error = ftc_snode_load( snode, cache->manager, gindex, &size ); } - FTC_CACHE_TRYLOOP_END(); + FTC_CACHE_TRYLOOP_END( list_changed ); ftcsnode->ref_count--; /* unlock the node */ @@ -406,9 +409,10 @@ FT_LOCAL_DEF( FT_Bool ) FTC_SNode_Compare( FTC_SNode snode, FTC_GQuery gquery, - FTC_Cache cache ) + FTC_Cache cache, + FT_Bool* list_changed ) { - return ftc_snode_compare( FTC_NODE( snode ), gquery, cache ); + return ftc_snode_compare( FTC_NODE( snode ), gquery, cache, list_changed ); } #endif diff --git a/src/cache/ftcsbits.h b/src/cache/ftcsbits.h index 336aa9b4c..5548149f9 100644 --- a/src/cache/ftcsbits.h +++ b/src/cache/ftcsbits.h @@ -88,7 +88,8 @@ FT_BEGIN_HEADER FT_LOCAL( FT_Bool ) FTC_SNode_Compare( FTC_SNode snode, FTC_GQuery gquery, - FTC_Cache cache ); + FTC_Cache cache, + FT_Bool* list_changed); #endif