From a mooc user: method source with it' take so long in Pharo 5

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

From a mooc user: method source with it' take so long in Pharo 5

stepharo
I am wondering why does the search 'method source with it' take so long
in Pharo 5? On my PC, When I select the text 'menu' and search for all
'method source with it', in Squeak 5 it takes 3 seconds. In Pharo 5 it
takes 21 seconds.


Reply | Threaded
Open this post in threaded view
|

Re: From a mooc user: method source with it' take so long in Pharo 5

CyrilFerlicot
Le 18/05/2016 19:12, stepharo a écrit :
> I am wondering why does the search 'method source with it' take so long
> in Pharo 5? On my PC, When I select the text 'menu' and search for all
> 'method source with it', in Squeak 5 it takes 3 seconds. In Pharo 5 it
> takes 21 seconds.
>
>

Did you try with the same computer? It depends a lot of the OS.
On my Mac it takes 20sec, on my windows it takes 7-8sec and on my linux
it takes 3-5sec.

--
Cyril Ferlicot

http://www.synectique.eu

165 Avenue Bretagne
Lille 59000 France


signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: From a mooc user: method source with it' take so long in Pharo 5

Nicolai Hess-3-2
In reply to this post by stepharo
Squeak caches the opened readonly file(handle). It does not have to reopen the file on every call for reading (readonly).

2016-05-18 19:12 GMT+02:00 stepharo <[hidden email]>:
I am wondering why does the search 'method source with it' take so long in Pharo 5? On my PC, When I select the text 'menu' and search for all 'method source with it', in Squeak 5 it takes 3 seconds. In Pharo 5 it takes 21 seconds.



Reply | Threaded
Open this post in threaded view
|

Re: From a mooc user: method source with it' take so long in Pharo 5

Mariano Martinez Peck


On Thu, May 19, 2016 at 4:30 AM, Nicolai Hess <[hidden email]> wrote:
Squeak caches the opened readonly file(handle). It does not have to reopen the file on every call for reading (readonly).


Some years ago I profiled some code of a customer, and there was a single line that was taking up 80% of the total time. Guess what? I was the opening of a temp / log file, which was opened (read only) each time... Caching the opened file was a HUGE speed up.

Cheers,


 
2016-05-18 19:12 GMT+02:00 stepharo <[hidden email]>:
I am wondering why does the search 'method source with it' take so long in Pharo 5? On my PC, When I select the text 'menu' and search for all 'method source with it', in Squeak 5 it takes 3 seconds. In Pharo 5 it takes 21 seconds.






--
Reply | Threaded
Open this post in threaded view
|

Re: From a mooc user: method source with it' take so long in Pharo 5

Henrik Nergaard
In reply to this post by Nicolai Hess-3-2

There is a method in SourceFileArray #localProcessReadStreamAtFileIndex:atPosition:ifPresent:ifAbsent: which uses a ProccessLocalVariable called ProccessAndSessionLocalSourcesFileArray (see #localProcessReadOnlyCopy). Changing the last line in #readStreamAt:ifPresent:ifAbsent: to use this local process one makes the time to run this snippet:

 

[ 1 systemNavigation browseMethodsWithSourceString: 'Morph' matchCase: false ] timeProfile

 

From ~ 10 seconds to ~ 3 seconds (Windows).

 

However I cannot see that the file handles created by this processLocalVariable to ever be closed, so I suspect those are leaked? In that case there might be the need to implement some “clean up” mechanism for ProccessLocalVariables before they are changed/nilled when a process changes.

 

Another approach could be to not use a ProccessLocalVariabe at all, but extend the SourceFilesArray class to also hold one read only handle for each of its files, and use these in readStreamAtFileIndex:::… . I guess that it is also necessary then to have semaphore protecting the two last lines such that setting the position in the stream and reading from it cannot be changed by other threads.

 

readStreamAtFileIndex: index atPosition: position ifPresent: presentBlock ifAbsent: absentBlock

 

               | stream  result|

               stream := self readOnlyFileAt: index.

               stream ifNil: [ ^ absentBlock value ].        "sources file not available"          

 

               position > stream size ifTrue: [ ^ absentBlock value ].

 

               readSema critical: [

                              stream position: position.

                              result := presentBlock value: stream

               ].

 

               ^ result

 

 

Best regards,

Henrik

 

 

From: Pharo-dev [mailto:[hidden email]] On Behalf Of Nicolai Hess
Sent: Thursday, May 19, 2016 9:31 AM
To: Pharo Development List <[hidden email]>
Subject: Re: [Pharo-dev] From a mooc user: method source with it' take so long in Pharo 5

 

Squeak caches the opened readonly file(handle). It does not have to reopen the file on every call for reading (readonly).

 

2016-05-18 19:12 GMT+02:00 stepharo <[hidden email]>:

I am wondering why does the search 'method source with it' take so long in Pharo 5? On my PC, When I select the text 'menu' and search for all 'method source with it', in Squeak 5 it takes 3 seconds. In Pharo 5 it takes 21 seconds.

 

Reply | Threaded
Open this post in threaded view
|

Re: From a mooc user: method source with it' take so long in Pharo 5

Eliot Miranda-2
In reply to this post by stepharo
Hi Steph,

    almost certainly because in Pharo the sources file is opened once for each method whereas in squeak a read-only copy is taken and reused.

I don't like the squeak approach:
    CurrentReadOnlySourceFiles cacheDuring:
        [...]
but it does play well with the debugger.  So fixing this elegantly is non-trivial.  I suggest you try and real squeak-dev messages talking about CurrentReadOnlySourceFiles and "read-only copies" of the sources files.

_,,,^..^,,,_ (phone)

> On May 18, 2016, at 10:12 AM, stepharo <[hidden email]> wrote:
>
> I am wondering why does the search 'method source with it' take so long in Pharo 5? On my PC, When I select the text 'menu' and search for all 'method source with it', in Squeak 5 it takes 3 seconds. In Pharo 5 it takes 21 seconds.
>
>

Reply | Threaded
Open this post in threaded view
|

Re: From a mooc user: method source with it' take so long in Pharo 5

Eliot Miranda-2
In reply to this post by Henrik Nergaard
Hi Henrik,

    cc'ing squeak-dev cuz this is relevant to both communities (if and until Pharo takes the unwise decision to double the size of the image by keeping sources in the image). 

On May 22, 2016, at 5:01 AM, Henrik Nergaard <[hidden email]> wrote:

There is a method in SourceFileArray #localProcessReadStreamAtFileIndex:atPosition:ifPresent:ifAbsent: which uses a ProccessLocalVariable called ProccessAndSessionLocalSourcesFileArray (see #localProcessReadOnlyCopy). Changing the last line in #readStreamAt:ifPresent:ifAbsent: to use this local process one makes the time to run this snippet:

 

[ 1 systemNavigation browseMethodsWithSourceString: 'Morph' matchCase: false ] timeProfile

 

From ~ 10 seconds to ~ 3 seconds (Windows).

 

However I cannot see that the file handles created by this processLocalVariable to ever be closed, so I suspect those are leaked? In that case there might be the need to implement some “clean up” mechanism for ProccessLocalVariables before they are changed/nilled when a process changes.

 

Another approach could be to not use a ProccessLocalVariabe at all, but extend the SourceFilesArray class to also hold one read only handle for each of its files, and use these in readStreamAtFileIndex:::… . I guess that it is also necessary then to have semaphore protecting the two last lines such that setting the position in the stream and reading from it cannot be changed by other threads.


I like this approach, but it has a fatal flaw.  If one is debugging file access, then as the debugger causes the source of methods to be fetched as the debugger displays them, so the file pointer in the read-only file the access of which one is trying to debug will change under one's feet, and the results will be completely wrong.

An approach which uses a special copy for the debugger doesn't work; one can't debug access to that file for the same reason.

An approach that /should/ work is for the debugger to install its own copy around file-access.  This should be recursive.  So around every file access in the debugger there would need to be something like

       SourceFiles substituteFreshReadOnlyCopiesDuring: [...file access...]

which would remember the current read-only files, evaluate the block using ensure: and have the ensure: block reinstall the previous file.

Does this make sense?

 

readStreamAtFileIndex: index atPosition: position ifPresent: presentBlock ifAbsent: absentBlock

 

               | stream  result|

               stream := self readOnlyFileAt: index.

               stream ifNil: [ ^ absentBlock value ].        "sources file not available"          

 

               position > stream size ifTrue: [ ^ absentBlock value ].

 

               readSema critical: [

                              stream position: position.

                              result := presentBlock value: stream

               ].

 

               ^ result

 

 

Best regards,

Henrik

 

 

From: Pharo-dev [[hidden email]] On Behalf Of Nicolai Hess
Sent: Thursday, May 19, 2016 9:31 AM
To: Pharo Development List <[hidden email]>
Subject: Re: [Pharo-dev] From a mooc user: method source with it' take so long in Pharo 5

 

Squeak caches the opened readonly file(handle). It does not have to reopen the file on every call for reading (readonly).

 

2016-05-18 19:12 GMT+02:00 stepharo <[hidden email]>:

I am wondering why does the search 'method source with it' take so long in Pharo 5? On my PC, When I select the text 'menu' and search for all 'method source with it', in Squeak 5 it takes 3 seconds. In Pharo 5 it takes 21 seconds.

 

Reply | Threaded
Open this post in threaded view
|

Re: From a mooc user: method source with it' take so long in Pharo 5

EstebanLM

On 22 May 2016, at 17:26, Eliot Miranda <[hidden email]> wrote:

Hi Henrik,

    cc'ing squeak-dev cuz this is relevant to both communities (if and until Pharo takes the unwise decision to double the size of the image by keeping sources in the image). 

this will not happen :)
yes, we have played with the idea, but at the end we have some good reasons to not do it… but by the way, it was not “doubling” because what we were keeping was a compressed version. 
What we’ll probably do is to change the file format to a real hash database for sources (like CDB, or something) and to epicea for changes (because epicea can store full refactor histories, etc…)… but this format migration is still very green, we need first to extract the functionality and decouple it (because now read/write is scattered all over the image).


On May 22, 2016, at 5:01 AM, Henrik Nergaard <[hidden email]> wrote:

There is a method in SourceFileArray #localProcessReadStreamAtFileIndex:atPosition:ifPresent:ifAbsent: which uses a ProccessLocalVariable called ProccessAndSessionLocalSourcesFileArray (see #localProcessReadOnlyCopy). Changing the last line in #readStreamAt:ifPresent:ifAbsent: to use this local process one makes the time to run this snippet:
 
[ 1 systemNavigation browseMethodsWithSourceString: 'Morph' matchCase: false ] timeProfile
 
From ~ 10 seconds to ~ 3 seconds (Windows).
 
However I cannot see that the file handles created by this processLocalVariable to ever be closed, so I suspect those are leaked? In that case there might be the need to implement some “clean up” mechanism for ProccessLocalVariables before they are changed/nilled when a process changes.
 
Another approach could be to not use a ProccessLocalVariabe at all, but extend the SourceFilesArray class to also hold one read only handle for each of its files, and use these in readStreamAtFileIndex:::… . I guess that it is also necessary then to have semaphore protecting the two last lines such that setting the position in the stream and reading from it cannot be changed by other threads.

I like this approach, but it has a fatal flaw.  If one is debugging file access, then as the debugger causes the source of methods to be fetched as the debugger displays them, so the file pointer in the read-only file the access of which one is trying to debug will change under one's feet, and the results will be completely wrong.

An approach which uses a special copy for the debugger doesn't work; one can't debug access to that file for the same reason.

An approach that /should/ work is for the debugger to install its own copy around file-access.  This should be recursive.  So around every file access in the debugger there would need to be something like

       SourceFiles substituteFreshReadOnlyCopiesDuring: [...file access...]

which would remember the current read-only files, evaluate the block using ensure: and have the ensure: block reinstall the previous file.

Does this make sense?

 
readStreamAtFileIndex: index atPosition: position ifPresent: presentBlock ifAbsent: absentBlock
 
               | stream  result|
               stream := self readOnlyFileAt: index.
               stream ifNil: [ ^ absentBlock value ].        "sources file not available"          
 
               position > stream size ifTrue: [ ^ absentBlock value ].
 
               readSema critical: [
                              stream position: position.
                              result := presentBlock value: stream
               ].
 
               ^ result
 
 
Best regards,
Henrik
 
 
From: Pharo-dev [[hidden email]] On Behalf Of Nicolai Hess
Sent: Thursday, May 19, 2016 9:31 AM
To: Pharo Development List <[hidden email]>
Subject: Re: [Pharo-dev] From a mooc user: method source with it' take so long in Pharo 5
 
Squeak caches the opened readonly file(handle). It does not have to reopen the file on every call for reading (readonly).
 
2016-05-18 19:12 GMT+02:00 stepharo <[hidden email]>:

I am wondering why does the search 'method source with it' take so long in Pharo 5? On my PC, When I select the text 'menu' and search for all 'method source with it', in Squeak 5 it takes 3 seconds. In Pharo 5 it takes 21 seconds.


Reply | Threaded
Open this post in threaded view
|

Re: From a mooc user: method source with it' take so long in Pharo 5

stepharo
In reply to this post by Eliot Miranda-2

Fun analysis :)



Le 22/5/16 à 17:26, Eliot Miranda a écrit :
Hi Henrik,

    cc'ing squeak-dev cuz this is relevant to both communities (if and until Pharo takes the unwise decision to double the size of the image by keeping sources in the image). 

On May 22, 2016, at 5:01 AM, Henrik Nergaard <[hidden email]> wrote:

There is a method in SourceFileArray #localProcessReadStreamAtFileIndex:atPosition:ifPresent:ifAbsent: which uses a ProccessLocalVariable called ProccessAndSessionLocalSourcesFileArray (see #localProcessReadOnlyCopy). Changing the last line in #readStreamAt:ifPresent:ifAbsent: to use this local process one makes the time to run this snippet:

 

[ 1 systemNavigation browseMethodsWithSourceString: 'Morph' matchCase: false ] timeProfile

 

From ~ 10 seconds to ~ 3 seconds (Windows).

 

However I cannot see that the file handles created by this processLocalVariable to ever be closed, so I suspect those are leaked? In that case there might be the need to implement some “clean up” mechanism for ProccessLocalVariables before they are changed/nilled when a process changes.

 

Another approach could be to not use a ProccessLocalVariabe at all, but extend the SourceFilesArray class to also hold one read only handle for each of its files, and use these in readStreamAtFileIndex:::… . I guess that it is also necessary then to have semaphore protecting the two last lines such that setting the position in the stream and reading from it cannot be changed by other threads.


I like this approach, but it has a fatal flaw.  If one is debugging file access, then as the debugger causes the source of methods to be fetched as the debugger displays them, so the file pointer in the read-only file the access of which one is trying to debug will change under one's feet, and the results will be completely wrong.

An approach which uses a special copy for the debugger doesn't work; one can't debug access to that file for the same reason.

An approach that /should/ work is for the debugger to install its own copy around file-access.  This should be recursive.  So around every file access in the debugger there would need to be something like

       SourceFiles substituteFreshReadOnlyCopiesDuring: [...file access...]

which would remember the current read-only files, evaluate the block using ensure: and have the ensure: block reinstall the previous file.

Does this make sense?

 

readStreamAtFileIndex: index atPosition: position ifPresent: presentBlock ifAbsent: absentBlock

 

               | stream  result|

               stream := self readOnlyFileAt: index.

               stream ifNil: [ ^ absentBlock value ].        "sources file not available"          

 

               position > stream size ifTrue: [ ^ absentBlock value ].

 

               readSema critical: [

                              stream position: position.

                              result := presentBlock value: stream

               ].

 

               ^ result

 

 

Best regards,

Henrik

 

 

From: Pharo-dev [[hidden email]] On Behalf Of Nicolai Hess
Sent: Thursday, May 19, 2016 9:31 AM
To: Pharo Development List <[hidden email]>
Subject: Re: [Pharo-dev] From a mooc user: method source with it' take so long in Pharo 5

 

Squeak caches the opened readonly file(handle). It does not have to reopen the file on every call for reading (readonly).

 

2016-05-18 19:12 GMT+02:00 stepharo <[hidden email]>:

I am wondering why does the search 'method source with it' take so long in Pharo 5? On my PC, When I select the text 'menu' and search for all 'method source with it', in Squeak 5 it takes 3 seconds. In Pharo 5 it takes 21 seconds.

 


Reply | Threaded
Open this post in threaded view
|

Re: From a mooc user: method source with it' take so long in Pharo 5

stepharo
In reply to this post by Eliot Miranda-2
tx



Le 22/5/16 à 17:17, Eliot Miranda a écrit :

> Hi Steph,
>
>      almost certainly because in Pharo the sources file is opened once for each method whereas in squeak a read-only copy is taken and reused.
>
> I don't like the squeak approach:
>      CurrentReadOnlySourceFiles cacheDuring:
>          [...]
> but it does play well with the debugger.  So fixing this elegantly is non-trivial.  I suggest you try and real squeak-dev messages talking about CurrentReadOnlySourceFiles and "read-only copies" of the sources files.
>
> _,,,^..^,,,_ (phone)
>
>> On May 18, 2016, at 10:12 AM, stepharo <[hidden email]> wrote:
>>
>> I am wondering why does the search 'method source with it' take so long in Pharo 5? On my PC, When I select the text 'menu' and search for all 'method source with it', in Squeak 5 it takes 3 seconds. In Pharo 5 it takes 21 seconds.
>>
>>
>


Reply | Threaded
Open this post in threaded view
|

Re: [squeak-dev] Re: From a mooc user: method source with it' take so long in Pharo 5

tinchodias
In reply to this post by Eliot Miranda-2
Hi all,

On Sun, May 22, 2016 at 12:26 PM, Eliot Miranda <[hidden email]> wrote:
Hi Henrik,

    cc'ing squeak-dev cuz this is relevant to both communities (if and until Pharo takes the unwise decision to double the size of the image by keeping sources in the image). 

On May 22, 2016, at 5:01 AM, Henrik Nergaard <[hidden email]> wrote:

There is a method in SourceFileArray #localProcessReadStreamAtFileIndex:atPosition:ifPresent:ifAbsent: which uses a ProccessLocalVariable called ProccessAndSessionLocalSourcesFileArray (see #localProcessReadOnlyCopy). Changing the last line in #readStreamAt:ifPresent:ifAbsent: to use this local process one makes the time to run this snippet:

 

[ 1 systemNavigation browseMethodsWithSourceString: 'Morph' matchCase: false ] timeProfile

 

From ~ 10 seconds to ~ 3 seconds (Windows).

 

However I cannot see that the file handles created by this processLocalVariable to ever be closed, so I suspect those are leaked? In that case there might be the need to implement some “clean up” mechanism for ProccessLocalVariables before they are changed/nilled when a process changes.

 

Another approach could be to not use a ProccessLocalVariabe at all, but extend the SourceFilesArray class to also hold one read only handle for each of its files, and use these in readStreamAtFileIndex:::… . I guess that it is also necessary then to have semaphore protecting the two last lines such that setting the position in the stream and reading from it cannot be changed by other threads.


I like this approach, but it has a fatal flaw.  If one is debugging file access, then as the debugger causes the source of methods to be fetched as the debugger displays them, so the file pointer in the read-only file the access of which one is trying to debug will change under one's feet, and the results will be completely wrong.

An approach which uses a special copy for the debugger doesn't work; one can't debug access to that file for the same reason.

An approach that /should/ work is for the debugger to install its own copy around file-access.  This should be recursive.  So around every file access in the debugger there would need to be something like

       SourceFiles substituteFreshReadOnlyCopiesDuring: [...file access...]

which would remember the current read-only files, evaluate the block using ensure: and have the ensure: block reinstall the previous file.

Does this make sense?


Interesting discussion. We could give a new try on the "local process read-only copy".
It was me who started with it last year in a Pharo sprint, but disabled it due to some errors... and didn't spend more time on it afterwards.

the reason to disable it: https://pharo.fogbugz.com/f/cases/15781/

none of them is very clear as documentation of the problem. 

 

readStreamAtFileIndex: index atPosition: position ifPresent: presentBlock ifAbsent: absentBlock

 

               | stream  result|

               stream := self readOnlyFileAt: index.

               stream ifNil: [ ^ absentBlock value ].        "sources file not available"          

 

               position > stream size ifTrue: [ ^ absentBlock value ].

 

               readSema critical: [

                              stream position: position.

                              result := presentBlock value: stream

               ].

 

               ^ result

 

 

Best regards,

Henrik

 

 

From: Pharo-dev [[hidden email]] On Behalf Of Nicolai Hess
Sent: Thursday, May 19, 2016 9:31 AM
To: Pharo Development List <[hidden email]>
Subject: Re: [Pharo-dev] From a mooc user: method source with it' take so long in Pharo 5

 

Squeak caches the opened readonly file(handle). It does not have to reopen the file on every call for reading (readonly).

 

2016-05-18 19:12 GMT+02:00 stepharo <[hidden email]>:

I am wondering why does the search 'method source with it' take so long in Pharo 5? On my PC, When I select the text 'menu' and search for all 'method source with it', in Squeak 5 it takes 3 seconds. In Pharo 5 it takes 21 seconds.

 





Reply | Threaded
Open this post in threaded view
|

Re: From a mooc user: method source with it' take so long in Pharo 5

Henrik Nergaard
In reply to this post by Eliot Miranda-2

I like this approach, but it has a fatal flaw.  If one is debugging file access, then as the debugger causes the source of methods to be fetched as the debugger displays them, so the file pointer in the read-only file the access of which one is trying to debug will change under one's feet, and the results will be completely wrong.

Ah yes, and trying to do something like:

SourceFiles readStreamAtFileIndex: 2 atPosition: 42 ifPresent: [ :stream | Halt now ] ifAbsent: [  ]

Crashes the image.

 

Here is another approach using a SharedQueue which caches any created read only copies up to a limit (5?). So for normal usage the queue will only hold 1 entry, but if there is concurrent work reading source files it might hold more so one do not have to always create a read only copy, if the first one is in use.

 

readStreamAtFileIndex: index atPosition: position ifPresent: presentBlock ifAbsent: absentBlock

 

               | stream rofa result |

              

               rofa := queue nextOrNil.

              

               rofa ifNil: [

                              [ rofa := files collect: [ :f | f readOnlyCopy ] ]

                                             on: FileDoesNotExistException

                                             do: [ ^ absentBlock value ]

].

              

               stream := rofa at: index.

 

               position > stream size ifTrue: [ ^ absentBlock value ].

               stream position: position.

               result := presentBlock value: stream.

              

               queue size < 5

                              ifTrue: [ queue nextPut: rofa ]

                              ifFalse: [ rofa do: [ :each | each close ] ].

                             

               ^ result

 

 

Best regards,

Henrik

 

 

From: Pharo-dev [mailto:[hidden email]] On Behalf Of Eliot Miranda
Sent: Sunday, May 22, 2016 5:27 PM
To: Pharo Development List <[hidden email]>
Cc: [hidden email]
Subject: Re: [Pharo-dev] From a mooc user: method source with it' take so long in Pharo 5

 

Hi Henrik,

    cc'ing squeak-dev cuz this is relevant to both communities (if and until Pharo takes the unwise decision to double the size of the image by keeping sources in the image). 


On May 22, 2016, at 5:01 AM, Henrik Nergaard <[hidden email]> wrote:

There is a method in SourceFileArray #localProcessReadStreamAtFileIndex:atPosition:ifPresent:ifAbsent: which uses a ProccessLocalVariable called ProccessAndSessionLocalSourcesFileArray (see #localProcessReadOnlyCopy). Changing the last line in #readStreamAt:ifPresent:ifAbsent: to use this local process one makes the time to run this snippet:

 

[ 1 systemNavigation browseMethodsWithSourceString: 'Morph' matchCase: false ] timeProfile

 

From ~ 10 seconds to ~ 3 seconds (Windows).

 

However I cannot see that the file handles created by this processLocalVariable to ever be closed, so I suspect those are leaked? In that case there might be the need to implement some “clean up” mechanism for ProccessLocalVariables before they are changed/nilled when a process changes.

 

Another approach could be to not use a ProccessLocalVariabe at all, but extend the SourceFilesArray class to also hold one read only handle for each of its files, and use these in readStreamAtFileIndex:::… . I guess that it is also necessary then to have semaphore protecting the two last lines such that setting the position in the stream and reading from it cannot be changed by other threads.

 

I like this approach, but it has a fatal flaw.  If one is debugging file access, then as the debugger causes the source of methods to be fetched as the debugger displays them, so the file pointer in the read-only file the access of which one is trying to debug will change under one's feet, and the results will be completely wrong.

 

An approach which uses a special copy for the debugger doesn't work; one can't debug access to that file for the same reason.

 

An approach that /should/ work is for the debugger to install its own copy around file-access.  This should be recursive.  So around every file access in the debugger there would need to be something like

 

       SourceFiles substituteFreshReadOnlyCopiesDuring: [...file access...]

 

which would remember the current read-only files, evaluate the block using ensure: and have the ensure: block reinstall the previous file.

 

Does this make sense?



 

readStreamAtFileIndex: index atPosition: position ifPresent: presentBlock ifAbsent: absentBlock

 

               | stream  result|

               stream := self readOnlyFileAt: index.

               stream ifNil: [ ^ absentBlock value ].        "sources file not available"          

 

               position > stream size ifTrue: [ ^ absentBlock value ].

 

               readSema critical: [

                              stream position: position.

                              result := presentBlock value: stream

               ].

 

               ^ result

 

 

Best regards,

Henrik

 

 

From: Pharo-dev [[hidden email]] On Behalf Of Nicolai Hess
Sent: Thursday, May 19, 2016 9:31 AM
To: Pharo Development List <[hidden email]>
Subject: Re: [Pharo-dev] From a mooc user: method source with it' take so long in Pharo 5

 

Squeak caches the opened readonly file(handle). It does not have to reopen the file on every call for reading (readonly).

 

2016-05-18 19:12 GMT+02:00 stepharo <[hidden email]>:

I am wondering why does the search 'method source with it' take so long in Pharo 5? On my PC, When I select the text 'menu' and search for all 'method source with it', in Squeak 5 it takes 3 seconds. In Pharo 5 it takes 21 seconds.


 

Reply | Threaded
Open this post in threaded view
|

Re: From a mooc user: method source with it' take so long in Pharo 5

Sven Van Caekenberghe-2

> On 05 Jun 2016, at 13:52, Henrik Nergaard <[hidden email]> wrote:
>
> I like this approach, but it has a fatal flaw.  If one is debugging file access, then as the debugger causes the source of methods to be fetched as the debugger displays them, so the file pointer in the read-only file the access of which one is trying to debug will change under one's feet, and the results will be completely wrong.
> Ah yes, and trying to do something like:
> SourceFiles readStreamAtFileIndex: 2 atPosition: 42 ifPresent: [ :stream | Halt now ] ifAbsent: [  ]
> Crashes the image.
>  
> Here is another approach using a SharedQueue which caches any created read only copies up to a limit (5?). So for normal usage the queue will only hold 1 entry, but if there is concurrent work reading source files it might hold more so one do not have to always create a read only copy, if the first one is in use.

This is really a cool and useful hack !

> readStreamAtFileIndex: index atPosition: position ifPresent: presentBlock ifAbsent: absentBlock
>  
>                | stream rofa result |
>              
>                rofa := queue nextOrNil.
>              
>                rofa ifNil: [
>                               [ rofa := files collect: [ :f | f readOnlyCopy ] ]
>                                              on: FileDoesNotExistException
>                                              do: [ ^ absentBlock value ]
> ].
>              
>                stream := rofa at: index.
>  
>                position > stream size ifTrue: [ ^ absentBlock value ].
>                stream position: position.
>                result := presentBlock value: stream.
>              
>                queue size < 5
>                               ifTrue: [ queue nextPut: rofa ]
>                               ifFalse: [ rofa do: [ :each | each close ] ].
>                              
>                ^ result
>  
>  
> Best regards,
> Henrik
>  
>  
> From: Pharo-dev [mailto:[hidden email]] On Behalf Of Eliot Miranda
> Sent: Sunday, May 22, 2016 5:27 PM
> To: Pharo Development List <[hidden email]>
> Cc: [hidden email]
> Subject: Re: [Pharo-dev] From a mooc user: method source with it' take so long in Pharo 5
>  
> Hi Henrik,
>
>     cc'ing squeak-dev cuz this is relevant to both communities (if and until Pharo takes the unwise decision to double the size of the image by keeping sources in the image).
>
> On May 22, 2016, at 5:01 AM, Henrik Nergaard <[hidden email]> wrote:
>
> There is a method in SourceFileArray #localProcessReadStreamAtFileIndex:atPosition:ifPresent:ifAbsent: which uses a ProccessLocalVariable called ProccessAndSessionLocalSourcesFileArray (see #localProcessReadOnlyCopy). Changing the last line in #readStreamAt:ifPresent:ifAbsent: to use this local process one makes the time to run this snippet:
>  
> [ 1 systemNavigation browseMethodsWithSourceString: 'Morph' matchCase: false ] timeProfile
>  
> From ~ 10 seconds to ~ 3 seconds (Windows).
>  
> However I cannot see that the file handles created by this processLocalVariable to ever be closed, so I suspect those are leaked? In that case there might be the need to implement some “clean up” mechanism for ProccessLocalVariables before they are changed/nilled when a process changes.
>  
> Another approach could be to not use a ProccessLocalVariabe at all, but extend the SourceFilesArray class to also hold one read only handle for each of its files, and use these in readStreamAtFileIndex:::… . I guess that it is also necessary then to have semaphore protecting the two last lines such that setting the position in the stream and reading from it cannot be changed by other threads.
>  
> I like this approach, but it has a fatal flaw.  If one is debugging file access, then as the debugger causes the source of methods to be fetched as the debugger displays them, so the file pointer in the read-only file the access of which one is trying to debug will change under one's feet, and the results will be completely wrong.
>  
> An approach which uses a special copy for the debugger doesn't work; one can't debug access to that file for the same reason.
>  
> An approach that /should/ work is for the debugger to install its own copy around file-access.  This should be recursive.  So around every file access in the debugger there would need to be something like
>  
>        SourceFiles substituteFreshReadOnlyCopiesDuring: [...file access...]
>  
> which would remember the current read-only files, evaluate the block using ensure: and have the ensure: block reinstall the previous file.
>  
> Does this make sense?
>
>
>  
> readStreamAtFileIndex: index atPosition: position ifPresent: presentBlock ifAbsent: absentBlock
>  
>                | stream  result|
>                stream := self readOnlyFileAt: index.
>                stream ifNil: [ ^ absentBlock value ].        "sources file not available"          
>  
>                position > stream size ifTrue: [ ^ absentBlock value ].
>  
>                readSema critical: [
>                               stream position: position.
>                               result := presentBlock value: stream
>                ].
>  
>                ^ result
>  
>  
> Best regards,
> Henrik
>  
>  
> From: Pharo-dev [mailto:[hidden email]] On Behalf Of Nicolai Hess
> Sent: Thursday, May 19, 2016 9:31 AM
> To: Pharo Development List <[hidden email]>
> Subject: Re: [Pharo-dev] From a mooc user: method source with it' take so long in Pharo 5
>  
> Squeak caches the opened readonly file(handle). It does not have to reopen the file on every call for reading (readonly).
>  
> 2016-05-18 19:12 GMT+02:00 stepharo <[hidden email]>:
> I am wondering why does the search 'method source with it' take so long in Pharo 5? On my PC, When I select the text 'menu' and search for all 'method source with it', in Squeak 5 it takes 3 seconds. In Pharo 5 it takes 21 seconds.
>


Reply | Threaded
Open this post in threaded view
|

Re: From a mooc user: method source with it' take so long in Pharo 5

Mariano Martinez Peck
In reply to this post by Henrik Nergaard
Hi Henrik,

I use the "method source with it" quite frequently (mostly when I am renaming methods or classes and I want to be sure those are not referenced from methods and class  comments and other cases) and so I would love some speedup. Would you mind opening an issue in the issue tracker and submitting an issue? Or at least sharing a .st ?
From what I can imagine is simply the code you pasted plus some #initialize that does "queue := SharedQueue new" ?

Thanks!
 

On Sun, Jun 5, 2016 at 8:52 AM, Henrik Nergaard <[hidden email]> wrote:

I like this approach, but it has a fatal flaw.  If one is debugging file access, then as the debugger causes the source of methods to be fetched as the debugger displays them, so the file pointer in the read-only file the access of which one is trying to debug will change under one's feet, and the results will be completely wrong.

Ah yes, and trying to do something like:

SourceFiles readStreamAtFileIndex: 2 atPosition: 42 ifPresent: [ :stream | Halt now ] ifAbsent: [  ]

Crashes the image.

 

Here is another approach using a SharedQueue which caches any created read only copies up to a limit (5?). So for normal usage the queue will only hold 1 entry, but if there is concurrent work reading source files it might hold more so one do not have to always create a read only copy, if the first one is in use.

 

readStreamAtFileIndex: index atPosition: position ifPresent: presentBlock ifAbsent: absentBlock

 

               | stream rofa result |

              

               rofa := queue nextOrNil.

              

               rofa ifNil: [

                              [ rofa := files collect: [ :f | f readOnlyCopy ] ]

                                             on: FileDoesNotExistException

                                             do: [ ^ absentBlock value ]

].

              

               stream := rofa at: index.

 

               position > stream size ifTrue: [ ^ absentBlock value ].

               stream position: position.

               result := presentBlock value: stream.

              

               queue size < 5

                              ifTrue: [ queue nextPut: rofa ]

                              ifFalse: [ rofa do: [ :each | each close ] ].

                             

               ^ result

 

 

Best regards,

Henrik

 

 

From: Pharo-dev [mailto:[hidden email]] On Behalf Of Eliot Miranda
Sent: Sunday, May 22, 2016 5:27 PM
To: Pharo Development List <[hidden email]>
Cc: [hidden email]


Subject: Re: [Pharo-dev] From a mooc user: method source with it' take so long in Pharo 5

 

Hi Henrik,

    cc'ing squeak-dev cuz this is relevant to both communities (if and until Pharo takes the unwise decision to double the size of the image by keeping sources in the image). 


On May 22, 2016, at 5:01 AM, Henrik Nergaard <[hidden email]> wrote:

There is a method in SourceFileArray #localProcessReadStreamAtFileIndex:atPosition:ifPresent:ifAbsent: which uses a ProccessLocalVariable called ProccessAndSessionLocalSourcesFileArray (see #localProcessReadOnlyCopy). Changing the last line in #readStreamAt:ifPresent:ifAbsent: to use this local process one makes the time to run this snippet:

 

[ 1 systemNavigation browseMethodsWithSourceString: 'Morph' matchCase: false ] timeProfile

 

From ~ 10 seconds to ~ 3 seconds (Windows).

 

However I cannot see that the file handles created by this processLocalVariable to ever be closed, so I suspect those are leaked? In that case there might be the need to implement some “clean up” mechanism for ProccessLocalVariables before they are changed/nilled when a process changes.

 

Another approach could be to not use a ProccessLocalVariabe at all, but extend the SourceFilesArray class to also hold one read only handle for each of its files, and use these in readStreamAtFileIndex:::… . I guess that it is also necessary then to have semaphore protecting the two last lines such that setting the position in the stream and reading from it cannot be changed by other threads.

 

I like this approach, but it has a fatal flaw.  If one is debugging file access, then as the debugger causes the source of methods to be fetched as the debugger displays them, so the file pointer in the read-only file the access of which one is trying to debug will change under one's feet, and the results will be completely wrong.

 

An approach which uses a special copy for the debugger doesn't work; one can't debug access to that file for the same reason.

 

An approach that /should/ work is for the debugger to install its own copy around file-access.  This should be recursive.  So around every file access in the debugger there would need to be something like

 

       SourceFiles substituteFreshReadOnlyCopiesDuring: [...file access...]

 

which would remember the current read-only files, evaluate the block using ensure: and have the ensure: block reinstall the previous file.

 

Does this make sense?



 

readStreamAtFileIndex: index atPosition: position ifPresent: presentBlock ifAbsent: absentBlock

 

               | stream  result|

               stream := self readOnlyFileAt: index.

               stream ifNil: [ ^ absentBlock value ].        "sources file not available"          

 

               position > stream size ifTrue: [ ^ absentBlock value ].

 

               readSema critical: [

                              stream position: position.

                              result := presentBlock value: stream

               ].

 

               ^ result

 

 

Best regards,

Henrik

 

 

From: Pharo-dev [[hidden email]] On Behalf Of Nicolai Hess
Sent: Thursday, May 19, 2016 9:31 AM
To: Pharo Development List <[hidden email]>
Subject: Re: [Pharo-dev] From a mooc user: method source with it' take so long in Pharo 5

 

Squeak caches the opened readonly file(handle). It does not have to reopen the file on every call for reading (readonly).

 

2016-05-18 19:12 GMT+02:00 stepharo <[hidden email]>:

I am wondering why does the search 'method source with it' take so long in Pharo 5? On my PC, When I select the text 'menu' and search for all 'method source with it', in Squeak 5 it takes 3 seconds. In Pharo 5 it takes 21 seconds.


 




--