<br><br><div class="gmail_quote">On Thu, Sep 8, 2011 at 2:18 PM, Levente Uzonyi <span dir="ltr"><<a href="mailto:leves@elte.hu">leves@elte.hu</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">On Thu, 8 Sep 2011, Eliot Miranda wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Levente, I object :) I find the findBinary:do:ifNone: extremely ugly and<br>
much prefer the findNearbyBinaryIndex: usage. Would you mind if we reverted<br>
to that?<br>
</blockquote>
<br></div>
I pushed it to the inbox, so there's nothing to revert. </blockquote><div><br></div><div>Ah, sorry. Saw the commit notice and assumed. Apologies.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
I prefer to have as few implementations of binary search as possible.<br></blockquote><div><br></div><div>Yes, but findNearbyBinaryIndex: pays its way. It saved 11 lines and is way more comprehensible.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Did you check the other changes? Are those acceptable?<br></blockquote><div><br></div><div>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.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><font color="#888888">
<br>
<br>
Levente<br>
</font><br>
P.S.: I just saw that Henrik wrote a mail about this method on the Pharo list with some other simplifications.<div><div></div><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
On Thu, Sep 8, 2011 at 2:04 PM, <<a href="mailto:commits@source.squeak.org" target="_blank">commits@source.squeak.org</a>> wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
A new version of Tools was added to project The Inbox:<br>
<a href="http://source.squeak.org/inbox/Tools-ul.378.mcz" target="_blank">http://source.squeak.org/<u></u>inbox/Tools-ul.378.mcz</a><br>
<br>
==================== Summary ====================<br>
<br>
Name: Tools-ul.378<br>
Author: ul<br>
Time: 8 September 2011, 10:57:10.107 pm<br>
UUID: 0262d60e-4ceb-b246-8d3f-<u></u>96424d2aa1ce<br>
Ancestors: Tools-eem.377<br>
<br>
Simplified DebuggerMethodMap >> #rangeForPC:<u></u>contextIsActiveContext:.<br>
- copy and sort the associations in a bit simpler and probably faster way<br>
- removed unnecessary checks (#isNil after non-nil assignment)<br>
- uses Array instead of SortedCollection :)<br>
- uses Juan's general purpose binary search instead of a custom one (it<br>
also handles all special cases)<br>
- uses #detectMax: instead of a specialized #inject:into:<br>
<br>
=============== Diff against Tools-eem.377 ===============<br>
<br>
Item was changed:<br>
----- Method: DebuggerMethodMap>>rangeForPC:<u></u>contextIsActiveContext: (in<br>
category 'source mapping') -----<br>
rangeForPC: contextsConcretePC contextIsActiveContext:<br>
contextIsActiveContext<br>
"Answer the indices in the source code for the supplied pc.<br>
If the context is the actve context (is at the hot end of the<br>
stack)<br>
then its pc is the current pc. But if the context isn't, because<br>
it is<br>
suspended sending a message, then its current pc is the previous<br>
pc."<br>
<br>
+ | pc |<br>
- | pc i end |<br>
pc := self method abstractPCForConcretePC: (contextIsActiveContext<br>
<br>
ifTrue: [contextsConcretePC]<br>
<br>
ifFalse: [(self method pcPreviousTo:<br>
contextsConcretePC)<br>
<br>
ifNotNil: [:prevpc|<br>
prevpc]<br>
<br>
ifNil:<br>
[contextsConcretePC]]).<br>
(self abstractSourceMap includesKey: pc) ifTrue:<br>
[^self abstractSourceMap at: pc].<br>
+ sortedSourceMap ifNil: [<br>
+ sortedSourceMap := self abstractSourceMap associations<br>
+ replace: [ :each | each copy ];<br>
+ sort ].<br>
+ ^sortedSourceMap<br>
+ findBinary: [ :assoc | pc - assoc key ]<br>
+ do: [ :item | item value ]<br>
+ ifNone: [ :lower :upper |<br>
+ lower<br>
+ ifNil: [ 1 to: 0 ]<br>
+ ifNotNil: [<br>
+ upper<br>
+ ifNotNil: [ upper value ]<br>
"No match, but a nearby element."<br>
+ ifNil: [<br>
+ | end |<br>
+ end :=<br>
sortedSourceMap detectMax: [ :each | each value last ].<br>
+ end + 1 to: end ] ]<br>
]<br>
- sortedSourceMap ifNil:<br>
- [sortedSourceMap := self abstractSourceMap.<br>
- sortedSourceMap := (sortedSourceMap keys collect:<br>
- [:key| key<br>
-> (sortedSourceMap at: key)]) asSortedCollection].<br>
- (sortedSourceMap isNil or: [sortedSourceMap isEmpty]) ifTrue: [^1<br>
to: 0].<br>
- i := sortedSourceMap findNearbyBinaryIndex: [:assoc| pc - assoc<br>
key].<br>
- i < 1 ifTrue: [^1 to: 0].<br>
- i > sortedSourceMap size ifTrue:<br>
- [end := sortedSourceMap inject: 0 into:<br>
- [:prev :this | prev max: this value last].<br>
- ^end+1 to: end].<br>
- ^(sortedSourceMap at: i) value<br>
<br>
"| method source scanner map |<br>
method := DebuggerMethodMap compiledMethodAt:<br>
#rangeForPC:<u></u>contextIsActiveContext:.<br>
source := method getSourceFromFile asString.<br>
scanner := InstructionStream on: method.<br>
map := method debuggerMap.<br>
Array streamContents:<br>
[:ranges|<br>
[scanner atEnd] whileFalse:<br>
[| range |<br>
range := map rangeForPC: scanner pc<br>
contextIsActiveContext: true.<br>
((map abstractSourceMap includesKey: scanner<br>
abstractPC)<br>
and: [range first ~= 0]) ifTrue:<br>
[ranges nextPut: (source copyFrom: range<br>
first to: range last)].<br>
scanner interpretNextInstructionFor:<br>
InstructionClient new]]"!<br>
<br>
<br>
<br>
</blockquote>
<br>
<br>
-- <br>
best,<br>
Eliot<br>
<br>
</blockquote>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br>best,<div>Eliot</div><br>