Re: [Pharo-dev] About the infinite debugger

[Eliot Miranda]

Hi Guille,

> On Jun 29, 2018, at 7:48 AM, Guillermo Polito  
> wrote:
> 
> Hi all,
> 
> during today's sprint we have been working with lots of people on the 
> infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We have 
> checked the emails sent in the latest month. Then, together with Quentin, 
> Pablo, Pavel, Yoan we have been discussing and testing hypothesis all day. We 
> have been also comparing the debuggers code between pharo 3/4 (where the bug 
> was is present) and pharo 7, but this was not necessarily straight forward as 
> the code is not the same and there is no easy diff...

This is frustrating.  I can’t see the issue cuz I can’t login to fogbugz.  
Having to login to read an issue is a major flaw.  I can see it makes sense for 
submitting, but fir merely browsing it should be unacceptable.  That said...

The pragma  actually sets the primitive number in the method, 
so it it not merely a pragma; it alters bits in the method that the VM uses to 
search for handler contexts.  So why one would do that for evaluateSignal: 
makes no sense to me. The primitive should be set only in on:do: or something 
very similar (for example one could imagine adding on:or:do: instead of using , 
to construct an ExceptionSet).  So I think removing it from evaluateSignal: is 
definitely the right thing to do.

As far as tests for findNextHandlerFrom:, this is tested implicitly by any 
nested exception test so I expect you have several tests affected.  Clément 
points to a test that fails when not including  in 
evaluateSignal: so more investigation is necessary.  Difficult to do while bugs 
are hidden in fogbugz.  When are they going to migrate the github where they 
belong?

> 
> ND, we have found that the problem may come from a wrong pragma marker. 
> Just removing that pragma gives us back the same behaviour as in  Pharo 3/4. 
> :D
> 
> https://github.com/pharo-project/pharo/pull/1621
> 
> I know that the exception handling/debugging has been modified several times 
> in the latest years (some refactorings, hiding contexts...), we unfortunately 
> don't have tests for it, so I'd like some more pair of eyes on it. Ben, 
> Martin could you take a look?
> 
> Thanks all for the fish,
> Guille

Re: [Pharo-dev] About the infinite debugger

[Esteban A. Maringolo]

Great news!

I hope this fixes it once and for all.

For some reason this bug bit me several times in several Pharo releases.

Thank you all for your dedication.

On 29/06/2018 11:48, Guillermo Polito wrote:
> Hi all,
> 
> during today's sprint we have been working with lots of people on the
> infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We
> have checked the emails sent in the latest month. Then, together with
> Quentin, Pablo, Pavel, Yoan we have been discussing and testing
> hypothesis all day. We have been also comparing the debuggers code
> between pharo 3/4 (where the bug was is present) and pharo 7, but this
> was not necessarily straight forward as the code is not the same and
> there is no easy diff...
> 
> ND, we have found that the problem may come from a wrong pragma
> marker. Just removing that pragma gives us back the same behaviour as
> in  Pharo 3/4. :D
> 
> https://github.com/pharo-project/pharo/pull/1621
> 
> I know that the exception handling/debugging has been modified several
> times in the latest years (some refactorings, hiding contexts...), we
> unfortunately don't have tests for it, so I'd like some more pair of
> eyes on it. Ben, Martin could you take a look?
> 
> Thanks all for the fish,
> Guille

-- 
Esteban A. Maringolo

Re: [Pharo-dev] About the infinite debugger

[Guillermo Polito]

On Fri, Jun 29, 2018 at 5:58 PM Ben Coman  wrote:

>
>
> On 29 June 2018 at 22:48, Guillermo Polito 
> wrote:
>
>> Hi all,
>>
>> during today's sprint we have been working with lots of people on the
>> infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We
>> have checked the emails sent in the latest month. Then, together with
>> Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis
>> all day. We have been also comparing the debuggers code between pharo 3/4
>> (where the bug was is present) and pharo 7, but this was not necessarily
>> straight forward as the code is not the same and there is no easy diff...
>>
>> ND, we have found that the problem may come from a wrong pragma
>> marker. Just removing that pragma gives us back the same behaviour as in
>> Pharo 3/4. :D
>>
>> https://github.com/pharo-project/pharo/pull/1621
>>
>> I know that the exception handling/debugging has been modified several
>> times in the latest years (some refactorings, hiding contexts...), we
>> unfortunately don't have tests for it, so I'd like some more pair of eyes
>> on it. Ben, Martin could you take a look?
>>
>> Thanks all for the fish,
>> Guille
>>
>
> I'm just about to fly out for a family vacation, so I'm not sure when I'll
> get a chance to have a good look.
> But I read the issue on Fogbugz and it echos some things I've been
> thinking recently,
> though I hadn't come close to that particular fix.
>
> I think my following comments are probably unrelated, but just sharing
> some thoughts that crossed my mind when I was looking into my own similar
> trouble.
> This is an area I'm not clear on and would like to understand better...
>
> When the UI thread (UI1) needs to be debugged (e.g. code run from
> Playground, or a test)
> and is suspended and a new UI thread (UI2) is started, which thread does
> the debugger run in?
>

Yes


> I guess it must run in UI2 but sometimes I got the vague feeling some
> parts of it were caught in UI1 and affected by UI1 being suspended.
>
But perhaps I got the wrong impression - its a complex interplay to follow.
>

Nope.. Actually, UI2 will simulate code of UI1 as it was running in UI1.
There is some machinery (look at #effectiveProcess) so that

Processor activeProcess

yields UI1 when stepping over UI2 code.


>
> The other vague notion I had was when thread UI1 is resumed, it seems to
> replace thread UI2 where the World keeps its,
> and wondered if  on the suspended-debug-UI1 thread somehow made
> it "resume" and so replace UI2 as the main thread,
> but then UI1 gets suspended again and the display freezes.
>
> Thanks for everyones efforts on this.
> cheers -ben
>


-- 



Guille Polito

Research Engineer

Centre de Recherche en Informatique, Signal et Automatique de Lille

CRIStAL - UMR 9189

French National Center for Scientific Research - *http://www.cnrs.fr
*


*Web:* *http://guillep.github.io* 

*Phone: *+33 06 52 70 66 13

Re: [Pharo-dev] [rmod] About the infinite debugger

[Guillermo Polito]

On Fri, Jun 29, 2018 at 5:54 PM Clément Bera  wrote:

> Hi,
>
> Normally the pull request should break the exception tests, especially the
> ones with nested handler, since they are the reason 199 marked signalling
> contexts were introduced
>

Yes, but it's ok that on:do: is marked but why evaluateSignal:? Do you know?


> .
>
> Looking quickly this test in latest Pharo 7 for example:
> ExceptionTests>>testHandlerContext
> fails with the pull request.
>
> Since the pull request changes a green test into a red test, I don't
> understand why you say "we unfortunately don't have tests for it".
>

Because I haven't see those. I was looking for tests about
#findNextHandlerContext, #findNextHandlerOrSignalingContext and I found
nothing.
But if we have some tests much better!


> I don't understand either why you say this pragma is "wrong", so I can't
> help with that either.
>

Well, I don't know if I have to repeat myself... This is apparently
provoking the infinite debugger issue. This pragma was not there in Pharo3,
and its not clear why a method that is not an exception handler is marked
as an exception handler. Is that clear enough?

For more info you can read the issue.


>
> Best,
>
> On Fri, Jun 29, 2018 at 4:48 PM, Guillermo Polito <
> guillermopol...@gmail.com> wrote:
>
>> Hi all,
>>
>> during today's sprint we have been working with lots of people on the
>> infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We
>> have checked the emails sent in the latest month. Then, together with
>> Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis
>> all day. We have been also comparing the debuggers code between pharo 3/4
>> (where the bug was is present) and pharo 7, but this was not necessarily
>> straight forward as the code is not the same and there is no easy diff...
>>
>> ND, we have found that the problem may come from a wrong pragma
>> marker. Just removing that pragma gives us back the same behaviour as in
>> Pharo 3/4. :D
>>
>> https://github.com/pharo-project/pharo/pull/1621
>>
>> I know that the exception handling/debugging has been modified several
>> times in the latest years (some refactorings, hiding contexts...), we
>> unfortunately don't have tests for it, so I'd like some more pair of eyes
>> on it. Ben, Martin could you take a look?
>>
>> Thanks all for the fish,
>> Guille
>>
>
>
>
> --
> Clément Béra
> https://clementbera.github.io/
> https://clementbera.wordpress.com/
>


-- 



Guille Polito

Research Engineer

Centre de Recherche en Informatique, Signal et Automatique de Lille

CRIStAL - UMR 9189

French National Center for Scientific Research - *http://www.cnrs.fr
*


*Web:* *http://guillep.github.io* 

*Phone: *+33 06 52 70 66 13

Re: [Pharo-dev] About the infinite debugger

[Ben Coman]

On 29 June 2018 at 22:48, Guillermo Polito 
wrote:

> Hi all,
>
> during today's sprint we have been working with lots of people on the
> infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We
> have checked the emails sent in the latest month. Then, together with
> Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis
> all day. We have been also comparing the debuggers code between pharo 3/4
> (where the bug was is present) and pharo 7, but this was not necessarily
> straight forward as the code is not the same and there is no easy diff...
>
> ND, we have found that the problem may come from a wrong pragma
> marker. Just removing that pragma gives us back the same behaviour as in
> Pharo 3/4. :D
>
> https://github.com/pharo-project/pharo/pull/1621
>
> I know that the exception handling/debugging has been modified several
> times in the latest years (some refactorings, hiding contexts...), we
> unfortunately don't have tests for it, so I'd like some more pair of eyes
> on it. Ben, Martin could you take a look?
>
> Thanks all for the fish,
> Guille
>

I'm just about to fly out for a family vacation, so I'm not sure when I'll
get a chance to have a good look.
But I read the issue on Fogbugz and it echos some things I've been thinking
recently,
though I hadn't come close to that particular fix.

I think my following comments are probably unrelated, but just sharing some
thoughts that crossed my mind when I was looking into my own similar
trouble.
This is an area I'm not clear on and would like to understand better...

When the UI thread (UI1) needs to be debugged (e.g. code run from
Playground, or a test)
and is suspended and a new UI thread (UI2) is started, which thread does
the debugger run in?
I guess it must run in UI2 but sometimes I got the vague feeling some parts
of it were caught in UI1 and affected by UI1 being suspended.
But perhaps I got the wrong impression - its a complex interplay to follow.

The other vague notion I had was when thread UI1 is resumed, it seems to
replace thread UI2 where the World keeps its,
and wondered if  on the suspended-debug-UI1 thread somehow made
it "resume" and so replace UI2 as the main thread,
but then UI1 gets suspended again and the display freezes.

Thanks for everyones efforts on this.
cheers -ben

Re: [Pharo-dev] [rmod] About the infinite debugger

[Clément Bera]

Hi,

Normally the pull request should break the exception tests, especially the
ones with nested handler, since they are the reason 199 marked signalling
contexts were introduced.

Looking quickly this test in latest Pharo 7 for example:
ExceptionTests>>testHandlerContext
fails with the pull request.

Since the pull request changes a green test into a red test, I don't
understand why you say "we unfortunately don't have tests for it". I don't
understand either why you say this pragma is "wrong", so I can't help with
that either.

Best,

On Fri, Jun 29, 2018 at 4:48 PM, Guillermo Polito  wrote:

> Hi all,
>
> during today's sprint we have been working with lots of people on the
> infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We
> have checked the emails sent in the latest month. Then, together with
> Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis
> all day. We have been also comparing the debuggers code between pharo 3/4
> (where the bug was is present) and pharo 7, but this was not necessarily
> straight forward as the code is not the same and there is no easy diff...
>
> ND, we have found that the problem may come from a wrong pragma
> marker. Just removing that pragma gives us back the same behaviour as in
> Pharo 3/4. :D
>
> https://github.com/pharo-project/pharo/pull/1621
>
> I know that the exception handling/debugging has been modified several
> times in the latest years (some refactorings, hiding contexts...), we
> unfortunately don't have tests for it, so I'd like some more pair of eyes
> on it. Ben, Martin could you take a look?
>
> Thanks all for the fish,
> Guille
>



-- 
Clément Béra
https://clementbera.github.io/
https://clementbera.wordpress.com/

[Pharo-dev] [Pharo 7.0-dev] Build #1104: 22174-Remove-FileDoesNotExistExceptiondefaultAction

[ci-pharo-ci-jenkins2]

There is a new Pharo build available!

The status of the build #1104 was: SUCCESS.

The Pull Request #1573 was integrated: 
"22174-Remove-FileDoesNotExistExceptiondefaultAction"
Pull request url: https://github.com/pharo-project/pharo/pull/1573

Issue Url: https://pharo.fogbugz.com/f/cases/22174
Build Url: 
https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/1104/

[Pharo-dev] About the infinite debugger

[Guillermo Polito]

Hi all,

during today's sprint we have been working with lots of people on the
infinite debugger problem (https://pharo.fogbugz.com/f/cases/22085/). We
have checked the emails sent in the latest month. Then, together with
Quentin, Pablo, Pavel, Yoan we have been discussing and testing hypothesis
all day. We have been also comparing the debuggers code between pharo 3/4
(where the bug was is present) and pharo 7, but this was not necessarily
straight forward as the code is not the same and there is no easy diff...

ND, we have found that the problem may come from a wrong pragma marker.
Just removing that pragma gives us back the same behaviour as in  Pharo
3/4. :D

https://github.com/pharo-project/pharo/pull/1621

I know that the exception handling/debugging has been modified several
times in the latest years (some refactorings, hiding contexts...), we
unfortunately don't have tests for it, so I'd like some more pair of eyes
on it. Ben, Martin could you take a look?

Thanks all for the fish,
Guille

[Pharo-dev] [Pharo 7.0-dev] Build #1103: 22112-Remove-code-related-to-HostSystemMenus

[ci-pharo-ci-jenkins2]

There is a new Pharo build available!

The status of the build #1103 was: FAILURE.

The Pull Request #1518 was integrated: 
"22112-Remove-code-related-to-HostSystemMenus"
Pull request url: https://github.com/pharo-project/pharo/pull/1518

Issue Url: https://pharo.fogbugz.com/f/cases/22112
Build Url: 
https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/1103/

[Pharo-dev] [Pharo 7.0-dev] Build #1102: 21844-Window-fullscreen-size-should-be-dependent-on-world-size

[ci-pharo-ci-jenkins2]

There is a new Pharo build available!

The status of the build #1102 was: SUCCESS.

The Pull Request #1617 was integrated: 
"21844-Window-fullscreen-size-should-be-dependent-on-world-size"
Pull request url: https://github.com/pharo-project/pharo/pull/1617

Issue Url: https://pharo.fogbugz.com/f/cases/21844
Build Url: 
https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/1102/

[Pharo-dev] [Pharo 7.0-dev] Build #1101: 20579-Ring-Core-Containers-seems-to-be-loaded-twice

[ci-pharo-ci-jenkins2]

There is a new Pharo build available!

The status of the build #1101 was: SUCCESS.

The Pull Request #1615 was integrated: 
"20579-Ring-Core-Containers-seems-to-be-loaded-twice"
Pull request url: https://github.com/pharo-project/pharo/pull/1615

Issue Url: https://pharo.fogbugz.com/f/cases/20579
Build Url: 
https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/1101/

[Pharo-dev] [Pharo 7.0-dev] Build #1100: 22058-TabMorph-should-handle-wheel-click-and-close-itself-like-in-most-software-as-web-browsers

[ci-pharo-ci-jenkins2]

There is a new Pharo build available!

The status of the build #1100 was: SUCCESS.

The Pull Request #1482 was integrated: 
"22058-TabMorph-should-handle-wheel-click-and-close-itself-like-in-most-software-as-web-browsers"
Pull request url: https://github.com/pharo-project/pharo/pull/1482

Issue Url: https://pharo.fogbugz.com/f/cases/22058
Build Url: 
https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/1100/

[Pharo-dev] [Pharo 7.0-dev] Build #1099: 22228-Class-copy-and-duplicateClassWithNewName-should-be-correctly-implemented

[ci-pharo-ci-jenkins2]

There is a new Pharo build available!

The status of the build #1099 was: SUCCESS.

The Pull Request #1613 was integrated: 
"8-Class-copy-and-duplicateClassWithNewName-should-be-correctly-implemented"
Pull request url: https://github.com/pharo-project/pharo/pull/1613

Issue Url: https://pharo.fogbugz.com/f/cases/8
Build Url: 
https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/1099/

[Pharo-dev] [Pharo 7.0-dev] Build #1098: 22196 Cleanup TraitsV2-Tests

[ci-pharo-ci-jenkins2]

There is a new Pharo build available!

The status of the build #1098 was: SUCCESS.

The Pull Request #1588 was integrated: "22196 Cleanup TraitsV2-Tests"
Pull request url: https://github.com/pharo-project/pharo/pull/1588

Issue Url: https://pharo.fogbugz.com/f/cases/22196
Build Url: 
https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/1098/

Re: [Pharo-dev] Why do we have SmallDictionary?

[Clément Bera]

Hum, about dictionary implementations, I've just found this [1]:
Open addressing vs. chaining
  *Chaining* *Open addressing*
*Collision resolution* Using external data structure Using hash table itself
*Memory waste* Pointer size overhead per entry (storing list heads in the
table) No overhead 1
*Performance dependence on table's load factor* Directly proportional
Proportional
to (loadFactor) / (1 - loadFactor)
*Allow to store more items, than hash table size* Yes No. Moreover, it's
recommended to keep table's load factor below 0.7
*Hash function requirements* Uniform distribution Uniform distribution,
should avoid clustering
*Handle removals* Removals are ok Removals clog the hash table with
"DELETED" entries
*Implementation* Simple Correct implementation of open addressing based
hash table is quite tricky1 Hash tables with chaining can work efficiently
even with load factor more than 1. At the same time, tables based on open
addressing scheme require load factor not to exceed 0.7 to be efficient.
Hence, 30% of slots remain empty, which leads to obvious memory waste.

Do you guys agree with this?

Their open addressing implementation is slightly different from the
Smalltalk one (they have deleted entries upon removal while we fix
collisions, at least in Pharo 6).
Their chaining implementation does not use balanced binary tree when a
bucket becomes large like Java's implementation.

I don't really understand why an open addressing implementation is trickier
to implement, in high level languages such as Smalltalk I may agree that it
might be easier to implement chaining, but in low level language such as in
the VM, IMO open addressing is easier since you've got only one large chunk
of memory to manage. Maybe it's because of hash collisions, having good
hash and good hash collection size (See HashTableSizes) is not that easy.

I'm curious because I do more and more bare metal programming and I end up
having to implement this kind of things myself, I've always implemented
naive open addressing up until now without really understanding details.

[1] http://www.algolist.net/Data_structures/Hash_table/Open_addressing

On Sat, Jun 16, 2018 at 8:49 PM, Max Leske  wrote:

>
>
> On 8 Jun 2018, at 09:48, Stéphane Rollandin wrote:
>
> FWIW it seems there is no SmallDictionary in Squeak.
>>
>
> Oh... Thanks Stéf, I wasn't aware of that.
>
>
>
> On 8 June 2018, at 15:13, John Brant wrote:
>
> Is anyone aware of a reason for hanging on to SmallDictionary? I'm also
 curious to know how SmallDictionary came to be. There must have been some
 advantage over Dictionary at some point in the past.

>>>
>>> It came from RB… the idea was that there are (in the Refactoring engine)
>>> a lot of very small dictionaries with <10 elements.
>>> The idea is that for such dictionaries, the overhead of hashing was
>>> higher than just linear search.
>>>
>>
>> I created its ancestor in VW some 20+ years ago (as RBSmallDictionary).
>> It was used when doing pattern matching. When it performs a pattern
>> match against an AST, it puts the potential value of the pattern
>> variable in the dictionary. If the value is used later in the pattern,
>> then we can get the previous value and make sure that we have an
>> equivalent AST. This allows you to write patterns like:
>>
>> `@a = `@a
>>
>> to find where someone has the same expression on both sides of the #=
>> message. Since most patterns have very few pattern variables, these
>> dictionaries won't hold many entries. Furthermore, we are likely to
>> abort the match when we have 0 entries.
>>
>> The original RBSmallDictionary had an #empty method that "emptied" the
>> dictionary without actually removing any elements -- it just set the
>> size to 0. In a general dictionary this would lead to memory leaks since
>> the previous values are still held by the dictionary. However, these
>> dictionaries were only used during the matching process and went away
>> after the process completed.
>>
>> Anyway, at the time when we converted our pattern matching code from
>> using the VW parser with our pattern matching extensions to use the new
>> RB parser with pattern matching, the time to run Smalllint on the image
>> was cut in half even though our parser was quite a bit slower than the
>> VW parser. I don't remember everything that was done, but I think that
>> most of the speedup came from having special pattern AST nodes and the
>> small dictionary.
>>
>>
>> John Brant
>>
>
>
> Very interesting! Thanks John!
>
> As Marcus has mentioned before in this thread, it would make a lot of
> sense to run benchmarks again. Actually, I think it would be nice to have a
> benchmark suite for these cases, that would let us monitor the performance
> and ensure that changes to the codebase don't have a deteriorative effect.
> I'm not saying that it would be easy to make this happen, writing proper
> benchmarks is hard (for me especially, as it seems, given my utter failure
> to think of the edge