On 05/26/2011 11:16 AM, Paolo Bonzini wrote:
On 05/26/2011 10:55 AM, Gwenael Casaccio wrote:
On 05/25/2011 04:17 PM, Gwenael Casaccio wrote:
On 05/25/2011 03:35 PM, Gwenael Casaccio wrote:
Hi,
There is a small bug in the class description in dict.c;
the super class of MethodDictionary should be IdentityDictionary.
Changing the superclass introduce a regression in Seaside (I've just
quickly fix the regression may be it's wrong)
Gwen
Hi again ;-)
I've updated the vm to support identitydictionary with ivars
commits in branch safeMthDic
(git://github.com/MrGwen/GNU-Smalltalk.git) :
9c1ac380352e5b5f8809
e872154d0f99360a2ba2
ccd4111352ca75ad78de
Gwen
And the last 0373c852bf7a0cf64660 :-)
Can you send a "compound" patch? I'm interested in this change too. Thanks!
Paolo
So here is the patch.
Gwen
diff --git a/kernel/MethodDict.st b/kernel/MethodDict.st
index b41304d..0d59f89 100644
--- a/kernel/MethodDict.st
+++ b/kernel/MethodDict.st
@@ -32,26 +32,34 @@
IdentityDictionary subclass: MethodDictionary [
-
+ | mutex |
+
<shape: #pointer>
<category: 'Language-Implementation'>
<comment: 'I am similar to an IdentityDictionary, except that removal and
rehashing operations inside my instances look atomic to the
interpreter.'>
+ mutex [
+ <category: 'initialization'>
+
+ ^ mutex ifNil: [ mutex := Semaphore forMutualExclusion ]
+ ]
+
at: key put: value [
"Store value as associated to the given key"
<category: 'adding'>
- | index |
- index := self findIndex: key.
- (self primAt: index) isNil
- ifTrue:
- [self incrementTally ifTrue: [index := self findIndex: key].
- self primAt: index put: key]
- ifFalse: [(self valueAt: index) discardTranslation].
- self valueAt: index put: value.
- Behavior flushCache.
+ self mutex critical: [
+ | index |
+ index := self findIndex: key.
+ (self primAt: index) isNil
+ ifTrue:
+ [self incrementTally ifTrue: [index := self findIndex: key].
+ self primAt: index put: key]
+ ifFalse: [(self valueAt: index) discardTranslation].
+ self valueAt: index put: value.
+ Behavior flushCache].
^value
]
@@ -64,12 +72,13 @@ interpreter.'>
<category: 'removing'>
| copy result |
- (self includesKey: anAssociation key)
- ifFalse: [SystemExceptions.NotFound signalOn: anAssociation key what: 'key'].
- copy := self copy.
- result := copy dangerouslyRemove: anAssociation.
- self become: copy.
- Behavior flushCache.
+ self mutex critical: [
+ (self includesKey: anAssociation key)
+ ifFalse: [SystemExceptions.NotFound signalOn: anAssociation key what: 'key'].
+ copy := self copy.
+ result := copy dangerouslyRemove: anAssociation.
+ self become: copy.
+ Behavior flushCache].
^result
]
@@ -83,11 +92,12 @@ interpreter.'>
<category: 'removing'>
| copy result |
- (self includesKey: anElement) ifFalse: [^aBlock value].
- copy := self copy.
- result := copy dangerouslyRemoveKey: anElement.
- self become: copy.
- Behavior flushCache.
+ self mutex critical: [
+ (self includesKey: anElement) ifFalse: [^aBlock value].
+ copy := self copy.
+ result := copy dangerouslyRemoveKey: anElement.
+ self become: copy.
+ Behavior flushCache].
^result
]
@@ -100,7 +110,7 @@ interpreter.'>
become that copy"
<category: 'rehashing'>
- self growBy: 0
+ self mutex critical: [ self growBy: 0 ]
]
dangerouslyRemove: anAssociation [
diff --git a/libgst/dict.c b/libgst/dict.c
index adb28f6..bb21c01 100644
--- a/libgst/dict.c
+++ b/libgst/dict.c
@@ -565,9 +565,9 @@ static const class_definition class_info[] = {
GST_ISP_POINTER, true, 0,
"IdentityDictionary", NULL, NULL, NULL },
- {&_gst_method_dictionary_class, &_gst_lookup_table_class,
- GST_ISP_POINTER, true, 0,
- "MethodDictionary", NULL, NULL, NULL },
+ {&_gst_method_dictionary_class, &_gst_identity_dictionary_class,
+ GST_ISP_POINTER, true, 1,
+ "MethodDictionary", "mutex", NULL, NULL },
/* These five MUST have the same structure as dictionary; they're
used interchangeably within the C portion of the system */
@@ -1398,9 +1398,10 @@ _gst_find_class_method (OOP class_oop,
OOP selector)
{
gst_class class;
- gst_identity_dictionary methodDictionary;
+ gst_object methodDictionary;
OOP method_dictionary_oop;
int index;
+ size_t numFixedFields;
class = (gst_class) OOP_TO_OBJ (class_oop);
method_dictionary_oop = class->methodDictionary;
@@ -1414,10 +1415,10 @@ _gst_find_class_method (OOP class_oop,
if (index < 0)
return (_gst_nil_oop);
- methodDictionary =
- (gst_identity_dictionary) OOP_TO_OBJ (method_dictionary_oop);
+ methodDictionary = OOP_TO_OBJ (method_dictionary_oop);
+ numFixedFields = OOP_FIXED_FIELDS (method_dictionary_oop);
- return (methodDictionary->keys[index]);
+ return (methodDictionary->data[index + numFixedFields]);
}
OOP
@@ -1587,45 +1588,43 @@ _gst_grow_dictionary (OOP oldDictionaryOOP)
return (OOP_TO_OBJ (oldDictionaryOOP));
}
-gst_identity_dictionary
+gst_object
_gst_grow_identity_dictionary (OOP oldIdentityDictionaryOOP)
{
- gst_identity_dictionary oldIdentityDictionary, identityDictionary;
+ gst_object oldIdentityDictionary, identityDictionary;
OOP key, identityDictionaryOOP;
- size_t oldNumFields, numFields, i, index;
+ size_t oldNumFields, numFields, numFixedFields, i, index;
- oldIdentityDictionary =
- (gst_identity_dictionary) OOP_TO_OBJ (oldIdentityDictionaryOOP);
- oldNumFields =
- (NUM_WORDS (oldIdentityDictionary) - 1) / 2;
+ oldIdentityDictionary = OOP_TO_OBJ (oldIdentityDictionaryOOP);
+ numFixedFields = OOP_FIXED_FIELDS (oldIdentityDictionaryOOP);
+ oldNumFields = (NUM_WORDS (oldIdentityDictionary) - numFixedFields) / 2;
numFields = new_num_fields (oldNumFields);
- identityDictionary = (gst_identity_dictionary)
+ identityDictionary =
instantiate_with (OOP_CLASS (oldIdentityDictionaryOOP), numFields * 2,
&identityDictionaryOOP);
- oldIdentityDictionary =
- (gst_identity_dictionary) OOP_TO_OBJ (oldIdentityDictionaryOOP);
+ oldIdentityDictionary = OOP_TO_OBJ (oldIdentityDictionaryOOP);
- identityDictionary->tally = oldIdentityDictionary->tally;
+ inst_var_at_put (identityDictionaryOOP, 1, inst_var_at (oldIdentityDictionaryOOP, 1));
/* rehash all associations from old dictionary into new one */
for (i = 0; i < oldNumFields; i++)
{
- key = oldIdentityDictionary->keys[i * 2];
+ key = oldIdentityDictionary->data[i * 2 + numFixedFields];
if COMMON (!IS_NIL (key))
{
index =
identity_dictionary_find_key_or_nil (identityDictionaryOOP,
key);
- identityDictionary->keys[index - 1] = key;
- identityDictionary->keys[index] = oldIdentityDictionary->keys[i*2+1];
+ identityDictionary->data[index - 1 + numFixedFields] = key;
+ identityDictionary->data[index + numFixedFields] = oldIdentityDictionary->data[i * 2 + 1 + numFixedFields];
}
}
_gst_swap_objects (identityDictionaryOOP, oldIdentityDictionaryOOP);
- return ((gst_identity_dictionary) OOP_TO_OBJ (oldIdentityDictionaryOOP));
+ return (OOP_TO_OBJ (oldIdentityDictionaryOOP));
}
@@ -1633,13 +1632,13 @@ ssize_t
identity_dictionary_find_key (OOP identityDictionaryOOP,
OOP keyOOP)
{
- gst_identity_dictionary identityDictionary;
- size_t index, count, numFields;
+ gst_object identityDictionary;
+ size_t index, count, numFields, numFixedFields;
- identityDictionary =
- (gst_identity_dictionary) OOP_TO_OBJ (identityDictionaryOOP);
+ identityDictionary = OOP_TO_OBJ (identityDictionaryOOP);
+ numFixedFields = OOP_FIXED_FIELDS (identityDictionaryOOP);
- numFields = NUM_WORDS (identityDictionary) - 1;
+ numFields = NUM_WORDS (identityDictionary) - numFixedFields;
index = scramble (OOP_INDEX (keyOOP)) * 2;
count = numFields / 2;
/* printf ("%d %d %O\n", count, index & numFields - 1, keyOOP); */
@@ -1647,10 +1646,10 @@ identity_dictionary_find_key (OOP identityDictionaryOOP,
{
index &= numFields - 1;
- if COMMON (IS_NIL (identityDictionary->keys[index]))
+ if COMMON (IS_NIL (identityDictionary->data[index + numFixedFields]))
return (-1);
- if COMMON (identityDictionary->keys[index] == keyOOP)
+ if COMMON (identityDictionary->data[index + numFixedFields] == keyOOP)
return (index + 1);
/* linear reprobe -- it is simple and guaranteed */
@@ -1669,13 +1668,13 @@ size_t
identity_dictionary_find_key_or_nil (OOP identityDictionaryOOP,
OOP keyOOP)
{
- gst_identity_dictionary identityDictionary;
- size_t index, count, numFields;
+ gst_object identityDictionary;
+ size_t index, count, numFields, numFixedFields;
- identityDictionary =
- (gst_identity_dictionary) OOP_TO_OBJ (identityDictionaryOOP);
+ identityDictionary = OOP_TO_OBJ (identityDictionaryOOP);
+ numFixedFields = OOP_FIXED_FIELDS (identityDictionaryOOP);
- numFields = NUM_WORDS (identityDictionary) - 1;
+ numFields = NUM_WORDS (identityDictionary) - numFixedFields;
index = scramble (OOP_INDEX (keyOOP)) * 2;
count = numFields / 2;
/* printf ("%d %d %O\n", count, index & numFields - 1, keyOOP); */
@@ -1683,10 +1682,10 @@ identity_dictionary_find_key_or_nil (OOP identityDictionaryOOP,
{
index &= numFields - 1;
- if COMMON (IS_NIL (identityDictionary->keys[index]))
+ if COMMON (IS_NIL (identityDictionary->data[index + numFixedFields]))
return (index + 1);
- if COMMON (identityDictionary->keys[index] == keyOOP)
+ if COMMON (identityDictionary->data[index + numFixedFields] == keyOOP)
return (index + 1);
/* linear reprobe -- it is simple and guaranteed */
@@ -1702,15 +1701,14 @@ identity_dictionary_find_key_or_nil (OOP identityDictionaryOOP,
OOP
identity_dictionary_new (OOP classOOP, int size)
{
- gst_identity_dictionary identityDictionary;
+ gst_object identityDictionary;
OOP identityDictionaryOOP;
size = new_num_fields (size);
- identityDictionary = (gst_identity_dictionary)
- instantiate_with (classOOP, size * 2, &identityDictionaryOOP);
+ identityDictionary = instantiate_with (classOOP, size * 2, &identityDictionaryOOP);
- identityDictionary->tally = FROM_INT (0);
+ inst_var_at_put (identityDictionaryOOP, 1, FROM_INT (0));
return (identityDictionaryOOP);
}
@@ -1719,31 +1717,32 @@ _gst_identity_dictionary_at_put (OOP identityDictionaryOOP,
OOP keyOOP,
OOP valueOOP)
{
- gst_identity_dictionary identityDictionary;
+ gst_object identityDictionary;
intptr_t index;
OOP oldValueOOP;
+ size_t numFixedFields;
- identityDictionary =
- (gst_identity_dictionary) OOP_TO_OBJ (identityDictionaryOOP);
+ identityDictionary = OOP_TO_OBJ (identityDictionaryOOP);
+ numFixedFields = OOP_FIXED_FIELDS (identityDictionaryOOP);
/* Never make dictionaries too full! For simplicity, we do this even
if the key is present in the dictionary (because it will most
likely resolve some collisions and make things faster). */
- if UNCOMMON (TO_INT (identityDictionary->tally) >=
+ if UNCOMMON (TO_INT (inst_var_at (identityDictionaryOOP, 1)) >=
TO_INT (identityDictionary->objSize) * 3 / 8)
identityDictionary =
- _gst_grow_identity_dictionary (identityDictionaryOOP);
+ (gst_object) _gst_grow_identity_dictionary (identityDictionaryOOP);
index =
identity_dictionary_find_key_or_nil (identityDictionaryOOP, keyOOP);
- if COMMON (IS_NIL (identityDictionary->keys[index - 1]))
- identityDictionary->tally = INCR_INT (identityDictionary->tally);
+ if COMMON (IS_NIL (identityDictionary->data[index - 1 + numFixedFields]))
+ inst_var_at_put (identityDictionaryOOP, 1, INCR_INT (inst_var_at (identityDictionaryOOP, 1)));
- identityDictionary->keys[index - 1] = keyOOP;
- oldValueOOP = identityDictionary->keys[index];
- identityDictionary->keys[index] = valueOOP;
+ identityDictionary->data[index - 1 + numFixedFields] = keyOOP;
+ oldValueOOP = identityDictionary->data[index + numFixedFields];
+ identityDictionary->data[index + numFixedFields] = valueOOP;
return (oldValueOOP);
}
@@ -1752,16 +1751,17 @@ OOP
_gst_identity_dictionary_at (OOP identityDictionaryOOP,
OOP keyOOP)
{
- gst_identity_dictionary identityDictionary;
+ gst_object identityDictionary;
intptr_t index;
+ size_t numFixedFields;
- identityDictionary =
- (gst_identity_dictionary) OOP_TO_OBJ (identityDictionaryOOP);
+ identityDictionary = OOP_TO_OBJ (identityDictionaryOOP);
+ numFixedFields = OOP_FIXED_FIELDS (identityDictionaryOOP);
index =
identity_dictionary_find_key_or_nil (identityDictionaryOOP, keyOOP);
- return identityDictionary->keys[index];
+ return identityDictionary->data[index + numFixedFields];
}
OOP
@@ -2206,18 +2206,19 @@ _gst_identity_dictionary_at_inc (OOP identityDictionaryOOP,
OOP keyOOP,
int inc)
{
- gst_identity_dictionary identityDictionary;
+ gst_object identityDictionary;
intptr_t index;
int oldValue;
+ size_t numFixedFields;
- identityDictionary =
- (gst_identity_dictionary) OOP_TO_OBJ (identityDictionaryOOP);
+ identityDictionary = OOP_TO_OBJ (identityDictionaryOOP);
+ numFixedFields = OOP_FIXED_FIELDS (identityDictionaryOOP);
/* Never make dictionaries too full! For simplicity, we do this even
if the key is present in the dictionary (because it will most
likely resolve some collisions and make things faster). */
- if UNCOMMON (TO_INT (identityDictionary->tally) >=
+ if UNCOMMON (TO_INT (inst_var_at (identityDictionaryOOP, 1)) >=
TO_INT (identityDictionary->objSize) * 3 / 8)
identityDictionary =
_gst_grow_identity_dictionary (identityDictionaryOOP);
@@ -2225,16 +2226,16 @@ _gst_identity_dictionary_at_inc (OOP identityDictionaryOOP,
index =
identity_dictionary_find_key_or_nil (identityDictionaryOOP, keyOOP);
- if UNCOMMON (IS_NIL (identityDictionary->keys[index - 1]))
+ if UNCOMMON (IS_NIL (identityDictionary->data[index - 1 + numFixedFields]))
{
- identityDictionary->tally = INCR_INT (identityDictionary->tally);
+ inst_var_at_put (identityDictionaryOOP, 1, INCR_INT (inst_var_at (identityDictionaryOOP, 1)));
oldValue = 0;
}
else
- oldValue = TO_INT(identityDictionary->keys[index]);
+ oldValue = TO_INT(identityDictionary->data[index + numFixedFields]);
- identityDictionary->keys[index - 1] = keyOOP;
- identityDictionary->keys[index] = FROM_INT(inc+oldValue);
+ identityDictionary->data[index - 1 + numFixedFields] = keyOOP;
+ identityDictionary->data[index + numFixedFields] = FROM_INT(inc+oldValue);
return (oldValue);
}
diff --git a/libgst/dict.h b/libgst/dict.h
index 087c5ae..f51058e 100644
--- a/libgst/dict.h
+++ b/libgst/dict.h
@@ -121,15 +121,6 @@ typedef struct gst_namespace
}
*gst_namespace;
-typedef struct gst_identity_dictionary
-{
- OBJ_HEADER;
- OOP tally; /* really, an int */
- OOP keys[1]; /* variable sized array of OOPS (keys/values) */
-}
- *gst_identity_dictionary;
-
-
#define BEHAVIOR_HEADER \
OBJ_HEADER; \
OOP superclass; \
@@ -588,7 +579,7 @@ extern gst_object _gst_grow_dictionary (OOP dictionaryOOP)
replaces the IdentityDictionary instance that IDENTITYDICTIONARYOOP
is pointing to with a new, larger dictionary, and returns this new
dictionary (the object pointer, not the OOP). */
-extern gst_identity_dictionary _gst_grow_identity_dictionary (OOP identityDictionaryOOP)
+extern gst_object _gst_grow_identity_dictionary (OOP identityDictionaryOOP)
ATTRIBUTE_HIDDEN;
/* Allocates and returns a new C (NULL-terminated) string that has the same
_______________________________________________
help-smalltalk mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/help-smalltalk
