On Thu, 8 Sep 2011, Eliot Miranda wrote:

> Levente,  I object :)  I find the findBinary:do:ifNone: extremely ugly and
> much prefer the findNearbyBinaryIndex: usage.  Would you mind if we reverted
> to that?

I pushed it to the inbox, so there's nothing to revert. I prefer to 
have as few implementations of binary search as possible.
Did you check the other changes? Are those acceptable?


Levente

P.S.: I just saw that Henrik wrote a mail about this method on the Pharo 
list with some other simplifications.

>
> On Thu, Sep 8, 2011 at 2:04 PM, <commits at source.squeak.org> wrote:
>
>> A new version of Tools was added to project The Inbox:
>> http://source.squeak.org/inbox/Tools-ul.378.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Tools-ul.378
>> Author: ul
>> Time: 8 September 2011, 10:57:10.107 pm
>> UUID: 0262d60e-4ceb-b246-8d3f-96424d2aa1ce
>> Ancestors: Tools-eem.377
>>
>> Simplified DebuggerMethodMap >> #rangeForPC:contextIsActiveContext:.
>> - copy and sort the associations in a bit simpler and probably faster way
>> - removed unnecessary checks (#isNil after non-nil assignment)
>> - uses Array instead of SortedCollection :)
>> - uses Juan's general purpose binary search instead of a custom one (it
>> also handles all special cases)
>> - uses #detectMax: instead of a specialized #inject:into:
>>
>> =============== Diff against Tools-eem.377 ===============
>>
>> Item was changed:
>>  ----- Method: DebuggerMethodMap>>rangeForPC:contextIsActiveContext: (in
>> category 'source mapping') -----
>>  rangeForPC: contextsConcretePC contextIsActiveContext:
>> contextIsActiveContext
>>        "Answer the indices in the source code for the supplied pc.
>>         If the context is the actve context (is at the hot end of the
>> stack)
>>         then its pc is the current pc.  But if the context isn't, because
>> it is
>>         suspended sending a message, then its current pc is the previous
>> pc."
>>
>> +       | pc |
>> -       | pc i end |
>>        pc := self method abstractPCForConcretePC: (contextIsActiveContext
>>
>>                            ifTrue: [contextsConcretePC]
>>
>>                            ifFalse: [(self method pcPreviousTo:
>> contextsConcretePC)
>>
>>                                                    ifNotNil: [:prevpc|
>> prevpc]
>>
>>                                                    ifNil:
>> [contextsConcretePC]]).
>>        (self abstractSourceMap includesKey: pc) ifTrue:
>>                [^self abstractSourceMap at: pc].
>> +       sortedSourceMap ifNil: [
>> +               sortedSourceMap := self abstractSourceMap associations
>> +                       replace: [ :each | each copy ];
>> +                       sort ].
>> +       ^sortedSourceMap
>> +               findBinary: [ :assoc | pc - assoc key ]
>> +               do: [ :item | item value ]
>> +               ifNone: [ :lower :upper |
>> +                       lower
>> +                               ifNil: [ 1 to: 0 ]
>> +                               ifNotNil: [
>> +                                       upper
>> +                                               ifNotNil: [ upper value ]
>> "No match, but a nearby element."
>> +                                               ifNil: [
>> +                                                       | end |
>> +                                                       end :=
>> sortedSourceMap detectMax: [ :each | each value last ].
>> +                                                       end + 1 to: end ] ]
>> ]
>> -       sortedSourceMap ifNil:
>> -               [sortedSourceMap := self abstractSourceMap.
>> -                sortedSourceMap := (sortedSourceMap keys collect:
>> -                                                               [:key| key
>> -> (sortedSourceMap at: key)]) asSortedCollection].
>> -       (sortedSourceMap isNil or: [sortedSourceMap isEmpty]) ifTrue: [^1
>> to: 0].
>> -       i := sortedSourceMap findNearbyBinaryIndex: [:assoc| pc - assoc
>> key].
>> -       i < 1 ifTrue: [^1 to: 0].
>> -       i > sortedSourceMap size ifTrue:
>> -               [end := sortedSourceMap inject: 0 into:
>> -                       [:prev :this | prev max: this value last].
>> -               ^end+1 to: end].
>> -       ^(sortedSourceMap at: i) value
>>
>>        "| method source scanner map |
>>         method := DebuggerMethodMap compiledMethodAt:
>> #rangeForPC:contextIsActiveContext:.
>>         source := method getSourceFromFile asString.
>>         scanner := InstructionStream on: method.
>>         map := method debuggerMap.
>>         Array streamContents:
>>                [:ranges|
>>                [scanner atEnd] whileFalse:
>>                        [| range |
>>                         range := map rangeForPC: scanner pc
>> contextIsActiveContext: true.
>>                         ((map abstractSourceMap includesKey: scanner
>> abstractPC)
>>                          and: [range first ~= 0]) ifTrue:
>>                                [ranges nextPut: (source copyFrom: range
>> first to: range last)].
>>                        scanner interpretNextInstructionFor:
>> InstructionClient new]]"!
>>
>>
>>
>
>
> -- 
> best,
> Eliot
>