Optimize compare of item in the factory

Bug #1495205 reported by hilaire on 2015-09-13
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Dr. Geo II
Medium
Unassigned

Bug Description

Here another x25 boost using hash value to compare mathItem:

DrGFactory>>indexOf: anItem
 ^ anItem
  ifNil: [0]
  ifNotNil: [
   self pool findFirst: [ :each |
    each hash = anItem hash ]
   "self pool indexOf: anItem"]

Changed in drgeo:
status: New → Fix Committed
Changed in drgeo:
milestone: wip → 15.12
Changed in drgeo:
status: Fix Committed → Fix Released
hilaire (hilaire-fernandes) wrote :

This optimisation breaks duplication check of newly created items.
For example, Segment with extremities swapped will be check as different models but there are the same.
It is because the hash value of the segment parents collection will be different.

parents1 = A, B
parents2 = B, A
=> parents1 hash ~= parents2 hash

Turning parents asSet produce two identical hash value,

May be some model should have parents collection as Set, or all models.
The implication of this change should be evaluated.

Changed in drgeo:
status: Fix Released → Confirmed
milestone: 15.12 → wip
hilaire (hilaire-fernandes) wrote :

This change have a limited impact and restore the model singularity check. It could be applied to other model, when needed.

DrGSegment2ptsItem>>rehash
 ^ hash := (self parents asSet hash bitXor: self nodeType hash) bitXor: self basicType hash

Changed in drgeo:
milestone: wip → 16.03
hilaire (hilaire-fernandes) wrote :

Applied the needed update rehash method to:
- Segment defined by two points
- Line defined by two points
- Arc defined by three points

Changed in drgeo:
status: Confirmed → Fix Committed
hilaire (hilaire-fernandes) wrote :

Using asSet in the rash is causing unwished hash collision.
Instead use the following rehash for line/segment defined by two points:

rehash
 ^ hash := super rehash bitXor: self parents reverse hash

Changed in drgeo:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers