On Thu, Sep 8, 2011 at 2:18 PM, Levente Uzonyi <leves at elte.hu> wrote:

> 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.


Ah, sorry.  Saw the commit notice and assumed.  Apologies.


> I prefer to have as few implementations of binary search as possible.
>

Yes, but findNearbyBinaryIndex: pays its way.  It saved 11 lines and is way
more comprehensible.


> Did you check the other changes? Are those acceptable?
>

I'm mulling them over :)  I like a variable for the context pc (which Henrik
didn't suggest) and I like using an at:ifAbsent: block.  So I'll hopefully
submit a new version sometime soon.


>
> 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<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
>>
>>
>


-- 
best,
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20110908/ccd6b632/attachment.htm