ApptiveGrid / Soil

An object oriented database that is easy to use and fun to play with
MIT License
45 stars 8 forks source link

SOTransaction idMap: use #at:ifPresent: #193

Closed MarcusDenker closed 1 year ago

MarcusDenker commented 1 year ago

The idMap is a dictionary objectID -> record, but we search it using #detect:ifFound:, which does a linear scan of the records:

objectWithId: objectId ifNone: aBlock
    idMap 
        detect: [ :each | each objectId = objectId ]
        ifFound: [ :record | ^ record object ].

    ^ self materializationWithId: objectId ifNone: aBlock.

We can use instead at:ifPresent:, to use the dictionary key and avoid linear scan:

objectWithId: objectId ifNone: aBlock
    idMap 
        at: objectId 
        ifPresent: [ :record | ^ record object ].

    ^ self materializationWithId: objectId ifNone: aBlock.
MarcusDenker commented 1 year ago

Note: objectId object might not be identical

MarcusDenker commented 1 year ago

So as it is not guaranteed that the objectID is identical, the current way is the only to make sure that it works correctly.

We could use a dictionary instead of an identityDict, but that would mean comparing equality all the time

Idea: implement a SoilOjectIDMap class that has a identity dictionary for each segment

noha commented 1 year ago

Why is it not guaranteed that ObjectId is identical? The design relies fully on the fact that it is. Most ObjectIds are allocated with an index of 0 which postpones the allocation of a new object id until committing. So there cannot be anything relying on equality because that would be bogus all the time. So an object being registered in a transaction gets a new objectId and is not addressable until it gets written to the database. On any other occurrence of looking up an object we can construct an objectId manually. Maybe there is a better way to do that

MarcusDenker commented 1 year ago

I checked the acceses of idMap: they are now all # at:ifPresent: and similar, no linear scan is done