May 2023 - Pharo-dev - lists.pharo.org

Re: Woden download from Github error

by Bernardo Ezequiel Contreras

i think there's an error in the baseline of Woden the project github://ronsaldo/sysmel has no master branch. it has a main branch. take a look at the stack trace. On Sat, May 20, 2023 at 12:46â¯AM Aik-Siong Koh <askoh(a)askoh.com> wrote: > Hi: > > I opened Pharo 9.0 - 64bit image. > Executed > > Metacello new > baseline: 'WodenEngine'; > repository: 'github://woden-engine/woden'; > load > > I got the following error: > NotFound: revspec 'master' not found > > Please advise. > Thanks, > Aik-Siong Koh > -- Bernardo E.C. Sent from a cheap desktop computer in South America.

May 20, 2023

Re: Woden download from Github error

by Aik-Siong Koh

Thanks. But I got the same error. Aik-Siong Milton Mamani Torres wrote: > You can try > Metacello new > baseline: 'WodenEngine'; > repository: 'github://woden-engine/woden:master'; > load > > El sÃ¡b, 20 may 2023 a las 5:45, Aik-Siong Koh (<askoh(a)askoh.com > <mailto:askoh@askoh.com>>) escribiÃ³: > > Hi: > > I opened Pharo 9.0 - 64bit image. > Executed > > Metacello new > baseline: 'WodenEngine'; > repository: 'github://woden-engine/woden'; > load > > I got the following error: > NotFound: revspec 'master' not found > > Please advise. > Thanks, > Aik-Siong Koh >

May 20, 2023

Re: The curious case of constant blocks

by stephane ducasse

Thanks marcus. I will turn this post into a doc fpr P12 > On 20 May 2023, at 11:02, Marcus Denker <marcus.denker(a)inria.fr> wrote: > > You might have come across code like this: > > ``` > minHeight > "answer the receiver's minHeight" > ^ self > valueOfProperty: #minHeight > ifAbsent: [2] > ``` > > In the case the #minHeight property is not set, it returns 2. > > Code like this is quite common, another example are empty ifAbsent blocks: > > > ``` > someDictonary remove: anObject ifAbsent: [] > ``` > > If we analyse the system, we can easily find all of them. The best is to use the AST for this: > > ``` > allBlocks := Smalltalk globals methods flatCollect: [:method | method ast blockNodes ]. > allBlocks size. "86805" > > nonInlinedBlocks := allBlocks select: [:blockNode | blockNode isInlined not]. > nonInlinedBlocks size. "36661" > > âthe blocks are actually just constant" > constantBlocks := nonInlinedBlocks select: [:blockNode | blockNode isConstant]. > constantBlocks size. "2572" > > ``` > > So there are 2572 constant (literal) blocks. You can inspect constantBlocks to explore them: > > <Constant.jpeg> > > Constant or empty blocks ([] is just [nil]) do not feel like something to think too much about. > > After all, they just retunr the literal when you send #value to them. What can be the problem? > > But: they are blocks, and in a system without clean blocks, they are full blocks, which means they are created at runtime for *every* execution of the [] block. And they are blocks, so there is a CompiledBlock created for each and sending #value will execute that bytecode, with the JIT having to create binary code. > > For Morph>>#minHeight the bytecode would be: > > ``` > "'49 <4C> self > 50 <20> pushConstant: #minHeight > 51 <F9 01 00> fullClosure:a CompiledBlock: [2] NumCopied: 0 > 54 <A2> send: valueOfProperty:ifAbsent: > 55 <5C> returnTop'" > ``` > > This is expensive! [2] is the same as 2 (the only thing we can do with the block is to send #value, and we can do that with the literal directly). > > ``` > [ 2 value ] bench. > [ [2] value ] bench > > 218625362.000/25750416.833 "8.490167884188363" > ``` > > So there >factor 8 for "create and evaluate" in difference between the two! > > This lead to people actually rewriting code to use the literal directly, e.g. we could just change it to > > > ``` > minHeight > "answer the receiver's minHeight" > ^ self > valueOfProperty: #minHeight > ifAbsent: 2 > ``` > > I am guilty of using this sometimes when optimizing for performance, but it does not feel nice. Yet another rule for performance to think about, and the number of constant blocks that are there shows that this is not how people want to do it. And, most important: it just works for 0 arg constant blocks, as literals undestand #value, but not #value:, #value:value: and so on. > > > So what can we do? The first thing (and I am sure you are thinking about that alreary) is the idea of clean blocks. Clean blocks are blocks that only need (to be created) information that the compiler has statically at compile time. you can look at RBProgramNode>>#isClean and the overrides in RBBlockNode>>#isClean RBVariableNode>>#isClean to see the exact cases, but for this case, all what you need to know is that a constant block, as it accesses nothing, is of course the trivial case of a clean block. > > If we compile them as clean blocks, we will immediatly move creation to compile time, and runtime property will be the same as using a literal. With the added benefit that constant blocks with arguments are supported, too. > > But: using "2" instead of [2] is not only faster for *creation*, it is faster when evaluting, too. The reason is that "2 value" sends #value, which executes Object>>#value, which is > > ``` > value > > ^self > ``` > > Which is a Quick return self method, aka a primitive: > > > ``` > self symbolic "'Quick return self'" > ``` > > This is *very fast*. While even as a clean block, we have, for every clean block, it's own method (compiledBlock) that the VM has to execute and thus create > code for: > > self symbolic > > "'25 <20> pushConstant: 2 > 26 <5E> blockReturn'" > > > It seems the fact that one is a quick return and the other a push/return is for the JIT not that of a difference, it matters for the interpreter more. But the JIT has to create code for *every* constant block, and #value means executing BlockClosure>>#value, which triggers execution of that compiedBlock. > > We thus have to execute two methods, not one. And the JIT has to cache all the generated code. > > So can we do better? It is actually easy to implement a class ConstantBlockClosure, subclass of CleanBlockClosure, that implements all the #value methods to just return > the constant value: > > ``` > value > ^literal > ``` > > Thus we get the same as with sending #value to the literal directly: we send #value, we execute one method that is a quick return. > > And the good news: there is #optionConstantBlockClosure in the compiler, and it is enabled by default in Pharo11! > > The reason why we can turn on Constant Bocks without problem is that they are never on the stack, so we do not need to take care to fix all the tools to know how to deal with them. > (Constant Bocks actually do have a CompiledBlock so that the e.g. for âsenders ofâ we check the literals just as if it would be a normal clean block, it is just never executed) > > If we go back to our method #minHeight, this means the bytecode looks like that: > > > ``` > self symbolic "'49 <4C> self > 50 <20> pushConstant: #minHeight > 51 <21> pushConstant: [2] > 52 <A2> send: valueOfProperty:ifAbsent: > 53 <5C> returnTop'" > ``` > > Thus, in Pharo11, the execution path of all the >2500 constant blocks end up executing one of the #value methods of ConstantBlockClosure. To get all the exceptions corect when sending e.g. #value ot a 1-arg block, there are subclasses for 1/2/3 args, we do not support 4 arg cleanBlocks for now (there are not many). > > If you want to check that this really works, go to ConstantBlockClosure>>#value and add a Counter via the Debug menu, it's really called a lot! > > Marcus > >

May 20, 2023

The curious case of constant blocks

by Marcus Denker

You might have come across code like this: ``` minHeight "answer the receiver's minHeight" ^ self valueOfProperty: #minHeight ifAbsent: [2] ``` In the case the #minHeight property is not set, it returns 2. Code like this is quite common, another example are empty ifAbsent blocks: ``` someDictonary remove: anObject ifAbsent: [] ``` If we analyse the system, we can easily find all of them. The best is to use the AST for this: ``` allBlocks := Smalltalk globals methods flatCollect: [:method | method ast blockNodes ]. allBlocks size. "86805" nonInlinedBlocks := allBlocks select: [:blockNode | blockNode isInlined not]. nonInlinedBlocks size. "36661" âthe blocks are actually just constant" constantBlocks := nonInlinedBlocks select: [:blockNode | blockNode isConstant]. constantBlocks size. "2572" ``` So there are 2572 constant (literal) blocks. You can inspect constantBlocks to explore them: Constant or empty blocks ([] is just [nil]) do not feel like something to think too much about. After all, they just retunr the literal when you send #value to them. What can be the problem? But: they are blocks, and in a system without clean blocks, they are full blocks, which means they are created at runtime for *every* execution of the [] block. And they are blocks, so there is a CompiledBlock created for each and sending #value will execute that bytecode, with the JIT having to create binary code. For Morph>>#minHeight the bytecode would be: ``` "'49 <4C> self 50 <20> pushConstant: #minHeight 51 <F9 01 00> fullClosure:a CompiledBlock: [2] NumCopied: 0 54 <A2> send: valueOfProperty:ifAbsent: 55 <5C> returnTop'" ``` This is expensive! [2] is the same as 2 (the only thing we can do with the block is to send #value, and we can do that with the literal directly). ``` [ 2 value ] bench. [ [2] value ] bench 218625362.000/25750416.833 "8.490167884188363" ``` So there >factor 8 for "create and evaluate" in difference between the two! This lead to people actually rewriting code to use the literal directly, e.g. we could just change it to ``` minHeight "answer the receiver's minHeight" ^ self valueOfProperty: #minHeight ifAbsent: 2 ``` I am guilty of using this sometimes when optimizing for performance, but it does not feel nice. Yet another rule for performance to think about, and the number of constant blocks that are there shows that this is not how people want to do it. And, most important: it just works for 0 arg constant blocks, as literals undestand #value, but not #value:, #value:value: and so on. So what can we do? The first thing (and I am sure you are thinking about that alreary) is the idea of clean blocks. Clean blocks are blocks that only need (to be created) information that the compiler has statically at compile time. you can look at RBProgramNode>>#isClean and the overrides in RBBlockNode>>#isClean RBVariableNode>>#isClean to see the exact cases, but for this case, all what you need to know is that a constant block, as it accesses nothing, is of course the trivial case of a clean block. If we compile them as clean blocks, we will immediatly move creation to compile time, and runtime property will be the same as using a literal. With the added benefit that constant blocks with arguments are supported, too. But: using "2" instead of [2] is not only faster for *creation*, it is faster when evaluting, too. The reason is that "2 value" sends #value, which executes Object>>#value, which is ``` value ^self ``` Which is a Quick return self method, aka a primitive: ``` self symbolic "'Quick return self'" ``` This is *very fast*. While even as a clean block, we have, for every clean block, it's own method (compiledBlock) that the VM has to execute and thus create code for: self symbolic "'25 <20> pushConstant: 2 26 <5E> blockReturn'" It seems the fact that one is a quick return and the other a push/return is for the JIT not that of a difference, it matters for the interpreter more. But the JIT has to create code for *every* constant block, and #value means executing BlockClosure>>#value, which triggers execution of that compiedBlock. We thus have to execute two methods, not one. And the JIT has to cache all the generated code. So can we do better? It is actually easy to implement a class ConstantBlockClosure, subclass of CleanBlockClosure, that implements all the #value methods to just return the constant value: ``` value ^literal ``` Thus we get the same as with sending #value to the literal directly: we send #value, we execute one method that is a quick return. And the good news: there is #optionConstantBlockClosure in the compiler, and it is enabled by default in Pharo11! The reason why we can turn on Constant Bocks without problem is that they are never on the stack, so we do not need to take care to fix all the tools to know how to deal with them. (Constant Bocks actually do have a CompiledBlock so that the e.g. for âsenders ofâ we check the literals just as if it would be a normal clean block, it is just never executed) If we go back to our method #minHeight, this means the bytecode looks like that: ``` self symbolic "'49 <4C> self 50 <20> pushConstant: #minHeight 51 <21> pushConstant: [2] 52 <A2> send: valueOfProperty:ifAbsent: 53 <5C> returnTop'" ``` Thus, in Pharo11, the execution path of all the >2500 constant blocks end up executing one of the #value methods of ConstantBlockClosure. To get all the exceptions corect when sending e.g. #value ot a 1-arg block, there are subclasses for 1/2/3 args, we do not support 4 arg cleanBlocks for now (there are not many). If you want to check that this really works, go to ConstantBlockClosure>>#value and add a Counter via the Debug menu, it's really called a lot! Marcus

May 20, 2023

Re: Woden download from Github error

by Milton Mamani Torres

You can try Metacello new baseline: 'WodenEngine'; repository: 'github://woden-engine/woden:master'; load El sÃ¡b, 20 may 2023 a las 5:45, Aik-Siong Koh (<askoh(a)askoh.com>) escribiÃ³: > Hi: > > I opened Pharo 9.0 - 64bit image. > Executed > > Metacello new > baseline: 'WodenEngine'; > repository: 'github://woden-engine/woden'; > load > > I got the following error: > NotFound: revspec 'master' not found > > Please advise. > Thanks, > Aik-Siong Koh >

May 20, 2023

Woden download from Github error

by Aik-Siong Koh

Hi: I opened Pharo 9.0 - 64bit image. Executed Metacello new baseline: 'WodenEngine'; repository: 'github://woden-engine/woden'; load I got the following error: NotFound: revspec 'master' not found Please advise. Thanks, Aik-Siong Koh

May 20, 2023

Re: Fun with CleanBlocks or: Speeding up Dictionary>>#at:

by Tim Mackinnon

I love these little discussions/examples that you share... things you take for granted have subtle impacts that are worth reasoning about. Whenever I read them, it definitely improves my understanding and makes me a better programmer. Thanks Markus. Tim On Fri, 19 May 2023, at 12:01 PM, Marcus Denker wrote: > Dictionary lookup is used a lot. Let's look at Dictionary>>#at: > > > at: key > "Answer the value associated with the key." > ^ self at: key ifAbsent: [self errorKeyNotFound: key] > > > The method looks simple and at a first glance, there are no obvious problems. > > But if we look at it again, we see that for every execution of #at:, > the block [self errorKeyNotFound: key] > has to be created. The method stores a CompiledBlock in the literals, > and a bytecode "create block" creates > the block: > > ``` > (Dictionary>>#at:) symbolic > > "'41 <4C> self > 42 <40> pushTemp: 0 > 43 <40> pushTemp: 0 > 44 <F9 00 01> fullClosure:a CompiledBlock: [self errorKeyNotFound: key] > NumCopied: 1 > 47 <A1> send: at:ifAbsent: > 48 <5C> returnTop'" > ``` > > This happens at *every* execution of #at:, even though the block that > we spend time to create will never > be executed at runtime (outside of real errors). > > If you look at the rest of the code path of #at: in a Dictionary, it is > carefully written to avoid block creation > by using optimized contructs (ifTrue: and friends). > > Could clean blocks help? Clean blocks are blocks that only use data > that the compiler knows at compile time, > thus we can create a CleanBlockClosure (which has the CompiledBlock) > and store *that* as a Literal. > > This then would mean that we could just use as fast pushLiteral: to > push the block, no creation at runtime needed. > > Clean blocks are not yet enabled by default, but we can use a compiler > option pragma to enable them just for this > method: > > ``` > <compilerOptions: #(+ optionCleanBlockClosure)> > ``` > > The problem is that the block references both "self" and the key to be > able to show a nice error message. > Referencing either one make the block not clean. > > Could we change the block to be clean? I guess we could not refer self > and inline the method #errorKeyNotFound: > (knowing the instance is not that important for logging the error in > production, for example). But we do want > to know the key that is not found, it really simplifies debugging > especially if you have to look at log files. > > If we closely look at the block: we know a bit more about it and how it > is used. We know that if it gets evaluated, > that evaluation will *always* happen with the homeContext of the block > on the stack, as it is will always be #at:ifAbsent: > that evaluates that block. > > And in that case, there is a trick that we can do: we can use > reflection to read the needed values via the stack. > > You might not be aware, but the debugger needs some quite interesting > features from the reflective layer of the system to provide > the user experience that you take for granted (and that feels trivial). > Imagine a block like that: > > ``` > tt > > | temp1 temp2 | > temp1 := 1. > temp2 := 2. > > self class methods do: [ :each | self halt. each with: temp1 ]. > ^ temp2 > ``` > > > The block does not reference temp2, so it actually is created as a > block that does not know temp2 at all. temp2 is not accessibe from the > block or it's context. > Yet, when debugging, you want to just be able to write temp2 in the > block and eval it (without saving the method), as *if* you would access > the temp in this block and recompile, the compiler would compile a > different block that would know temp2. > > So the reflective API of Context (and the infrastructure of reading > Variables reflectively), is build in a way that #readVariableNamed: > will > use the stack to find the value of temps that are not available in the > block context, but could be available if they would be referenced > statically. > > And if you think about it, we have just a case like that here, in > reverse: if we would use #readVariableNamed: on the context to read the > argument, > the compiler would not see a read of "key", thus compile a block that > does not have that temp available. And if we then read "self" via > thisContext, too, > the compiler sees a block that it can compile as a clean block: > > > at: key > "Answer the value associated with the key." > > <compilerOptions: #(+ optionCleanBlockClosure)> > > ^ self at: key ifAbsent: [ > > "this block is never executed, yet we pay runtime cost to create it > if it is a full block. > We use instead the reflective API to read the argument and receiver > via the stack, making > the block clean. > > We enable cleanBlocks for this method by setting > optionCleanBlockClosure as it is not yet > enabled by default" > > KeyNotFound signalFor: (thisContext readVariableNamed: #key) in: > thisContext sender receiver > "This is equivalent to: > self errorKeyNotFound: key" > ] > > > Let's look at the printed symbolic bytecode: > > > ``` > Dictionary>>#at:) symbolic > > "'41 <4C> self > 42 <40> pushTemp: 0 > 43 <20> pushConstant: [ > > "this block is never executed, yet we pay runtime cost to create it > if it is a full block. > We use instead the reflective API to read the argument and receiver > via the stack, making > the block clean. > > We enable cleanBlocks for this method by setting > optionCleanBlockClosure as it is not yet > enabled by default" > > KeyNotFound signalFor: (thisContext readVariableNamed: #key) in: > thisContext sender receiver > "This is equivalent to: > self errorKeyNotFound: key" > ] > 44 <A1> send: at:ifAbsent: > 45 <5C> returnTop'" > ``` > > you see that it now uses pushConstant: to push the pre-compiled > CleanBlockClosure, which is *much* faster. > > A very naive benchmark using a fairly large Dictionary (Smalltalk globals): > > ``` > [Smalltalk globals at: #Object] bench > > 8591996.801/7040335.266 "1.220395972119888" > ``` > > Shows a speedup of ~20%, which is qute a lot! > > Of course, this is a hack. For one, it just solves this for this one > case. And we do not want to add code like > that everywhere. And real world impact of speed will be quite limited > due to that (it just speeds up > Dictionary>#at:) > > The alternative would be to inline #at:ifAbsent: This inlining, due to > the existing subclasses, leads to quite some > needed refactorings, and has the same problem of being a solution for > this problem in this one place. > > The real solution in the long term is a JIT that removes blocks like > that by inlining automatically for the code it executes, > while not changing the image level code at all. This would then solve > this for all similar cases everywhere all at once. > > But the nice aspect of the hack is: it is local (touches just this one > method), gives us some speedup now and will be > trivially removable when we deploy a better solution in the future. > > But the code is quite ugly and indeed relies on reflection... which > maybe for Dictionary>>#at: is not that nice > (e.g. if you want to create a minimal image without reflection) > > But it is save to add this to your own image, it works in Pharo11 and > Pharo12 and I guess even in Pharo10 > (not tested there). > > Marcus

May 19, 2023

This week (20/2023) on the Pharo Issue Tracker.

by Marcus Denker

This week, we continued the work on Pharo12. #Fixes - Drop use of Paragraph from #drawOn: in ToggleWithSymbolMenuItemShortcut to fix menu item alignment #13764 https://github.com/pharo-project/pharo/pull/13764 - 13657-With-CleanBlocks-enabled-tests-fail-when-trying-to-Fuel-out-stack #13763 https://github.com/pharo-project/pharo/pull/13763 - Adjust icon and key text spacing in ToggleMenuItemMorph #13722 https://github.com/pharo-project/pharo/pull/13722 # Features - add object to represent the Native Window #13566 https://github.com/pharo-project/pharo/pull/13566 - Add menu items âIndentâ and âOutdentâ to the menu built through #menuOn: in RubSmalltalkCodeMode #13714 https://github.com/pharo-project/pharo/pull/13714 # Compiler - Remove Object bindingOf: and hasBindingOf: #13736 https://github.com/pharo-project/pharo/pull/13736 https://github.com/pharo-spec/Spec/pull/1392 - Compiler: remove #lookupVar:declare: #13737 https://github.com/pharo-project/pharo/pull/13737 https://github.com/pharo-spec/Spec/pull/1390 - 13723-Binding-for-variable-in-Playground-is-nil-after-assigning-a-number-to-it #13739 https://github.com/pharo-project/pharo/pull/13739 # Protocols - Method announcements should use real protocols #13730 https://github.com/pharo-project/pharo/pull/13730 - Clean the way to get the selectors in a protocol #13759 https://github.com/pharo-project/pharo/pull/13759 - Inline protocol removal from ClassOrganization #13753 https://github.com/pharo-project/pharo/pull/13753 - Diver cleanings around protocols #13749 https://github.com/pharo-project/pharo/pull/13749 - Inline ClassOrganization>>#removeElement: in ClassDescription #13748 https://github.com/pharo-project/pharo/pull/13748 - Implement ClassDescription>>#protocolNameOfSelector: #13728 https://github.com/pharo-project/pharo/pull/13728 - Move #protocolOfSelector: to ClassDescription #13711 https://github.com/pharo-project/pharo/pull/13711 - Improve removal of protocols and add tests #13692 https://github.com/pharo-project/pharo/pull/13692 # Refactoring Engine - Refactoring RBRefactoringWarning and RBRefactoringError #13367 https://github.com/pharo-project/pharo/pull/13367 # Cleanup - Update 01-loadMetacello.st: MetacelloPharoCommonPlatform #13762 https://github.com/pharo-project/pharo/pull/13762 - update-dependencies #13754 https://github.com/pharo-project/pharo/pull/13754 - 10069-Review-NullTextStyler #13752 https://github.com/pharo-project/pharo/pull/13752 - 12444-cleanup-todo-from-testClassSideInitializeMethodNeedsToBeInClassInitializationProtocol #13750 https://github.com/pharo-project/pharo/pull/13750 - Cleanup-MetacelloPharoPlatform #13738 https://github.com/pharo-project/pharo/pull/13738 https://github.com/pharo-project/pharo/pull/13747 https://github.com/pharo-project/pharo/pull/13716 - Move-isObservableSlot #13745 https://github.com/pharo-project/pharo/pull/13745 https://github.com/pharo-spec/Spec/pull/1393 - Add class comments to mocks #13743 https://github.com/pharo-project/pharo/pull/13743 - Class-Deprecate-classComment #13744 https://github.com/pharo-project/pharo/pull/13744 - Cleanup TheManifestBuilder #13742 https://github.com/pharo-project/pharo/pull/13742 - finally remove AbstractTool #13715 https://github.com/pharo-project/pharo/pull/13715 https://github.com/pharo-spec/NewTools/pull/523 - Remove dead code in session manager #13729 https://github.com/pharo-project/pharo/pull/13729 - Fix deprecated call #13710 https://github.com/pharo-project/pharo/pull/13710 - extract return from ifNilifNotNil in ChangeSorterModel #516 https://github.com/pharo-spec/NewTools/pull/516 - Do not listen to method recategorization in MessageBrowser #524 https://github.com/pharo-spec/NewTools/pull/524

May 19, 2023

Fun with CleanBlocks or: Speeding up Dictionary>>#at:

by Marcus Denker

Dictionary lookup is used a lot. Let's look at Dictionary>>#at: at: key "Answer the value associated with the key." ^ self at: key ifAbsent: [self errorKeyNotFound: key] The method looks simple and at a first glance, there are no obvious problems. But if we look at it again, we see that for every execution of #at:, the block [self errorKeyNotFound: key] has to be created. The method stores a CompiledBlock in the literals, and a bytecode "create block" creates the block: ``` (Dictionary>>#at:) symbolic "'41 <4C> self 42 <40> pushTemp: 0 43 <40> pushTemp: 0 44 <F9 00 01> fullClosure:a CompiledBlock: [self errorKeyNotFound: key] NumCopied: 1 47 <A1> send: at:ifAbsent: 48 <5C> returnTop'" ``` This happens at *every* execution of #at:, even though the block that we spend time to create will never be executed at runtime (outside of real errors). If you look at the rest of the code path of #at: in a Dictionary, it is carefully written to avoid block creation by using optimized contructs (ifTrue: and friends). Could clean blocks help? Clean blocks are blocks that only use data that the compiler knows at compile time, thus we can create a CleanBlockClosure (which has the CompiledBlock) and store *that* as a Literal. This then would mean that we could just use as fast pushLiteral: to push the block, no creation at runtime needed. Clean blocks are not yet enabled by default, but we can use a compiler option pragma to enable them just for this method: ``` <compilerOptions: #(+ optionCleanBlockClosure)> ``` The problem is that the block references both "self" and the key to be able to show a nice error message. Referencing either one make the block not clean. Could we change the block to be clean? I guess we could not refer self and inline the method #errorKeyNotFound: (knowing the instance is not that important for logging the error in production, for example). But we do want to know the key that is not found, it really simplifies debugging especially if you have to look at log files. If we closely look at the block: we know a bit more about it and how it is used. We know that if it gets evaluated, that evaluation will *always* happen with the homeContext of the block on the stack, as it is will always be #at:ifAbsent: that evaluates that block. And in that case, there is a trick that we can do: we can use reflection to read the needed values via the stack. You might not be aware, but the debugger needs some quite interesting features from the reflective layer of the system to provide the user experience that you take for granted (and that feels trivial). Imagine a block like that: ``` tt | temp1 temp2 | temp1 := 1. temp2 := 2. self class methods do: [ :each | self halt. each with: temp1 ]. ^ temp2 ``` The block does not reference temp2, so it actually is created as a block that does not know temp2 at all. temp2 is not accessibe from the block or it's context. Yet, when debugging, you want to just be able to write temp2 in the block and eval it (without saving the method), as *if* you would access the temp in this block and recompile, the compiler would compile a different block that would know temp2. So the reflective API of Context (and the infrastructure of reading Variables reflectively), is build in a way that #readVariableNamed: will use the stack to find the value of temps that are not available in the block context, but could be available if they would be referenced statically. And if you think about it, we have just a case like that here, in reverse: if we would use #readVariableNamed: on the context to read the argument, the compiler would not see a read of "key", thus compile a block that does not have that temp available. And if we then read "self" via thisContext, too, the compiler sees a block that it can compile as a clean block: at: key "Answer the value associated with the key." <compilerOptions: #(+ optionCleanBlockClosure)> ^ self at: key ifAbsent: [ "this block is never executed, yet we pay runtime cost to create it if it is a full block. We use instead the reflective API to read the argument and receiver via the stack, making the block clean. We enable cleanBlocks for this method by setting optionCleanBlockClosure as it is not yet enabled by default" KeyNotFound signalFor: (thisContext readVariableNamed: #key) in: thisContext sender receiver "This is equivalent to: self errorKeyNotFound: key" ] Let's look at the printed symbolic bytecode: ``` Dictionary>>#at:) symbolic "'41 <4C> self 42 <40> pushTemp: 0 43 <20> pushConstant: [ "this block is never executed, yet we pay runtime cost to create it if it is a full block. We use instead the reflective API to read the argument and receiver via the stack, making the block clean. We enable cleanBlocks for this method by setting optionCleanBlockClosure as it is not yet enabled by default" KeyNotFound signalFor: (thisContext readVariableNamed: #key) in: thisContext sender receiver "This is equivalent to: self errorKeyNotFound: key" ] 44 <A1> send: at:ifAbsent: 45 <5C> returnTop'" ``` you see that it now uses pushConstant: to push the pre-compiled CleanBlockClosure, which is *much* faster. A very naive benchmark using a fairly large Dictionary (Smalltalk globals): ``` [Smalltalk globals at: #Object] bench 8591996.801/7040335.266 "1.220395972119888" ``` Shows a speedup of ~20%, which is qute a lot! Of course, this is a hack. For one, it just solves this for this one case. And we do not want to add code like that everywhere. And real world impact of speed will be quite limited due to that (it just speeds up Dictionary>#at:) The alternative would be to inline #at:ifAbsent: This inlining, due to the existing subclasses, leads to quite some needed refactorings, and has the same problem of being a solution for this problem in this one place. The real solution in the long term is a JIT that removes blocks like that by inlining automatically for the code it executes, while not changing the image level code at all. This would then solve this for all similar cases everywhere all at once. But the nice aspect of the hack is: it is local (touches just this one method), gives us some speedup now and will be trivially removable when we deploy a better solution in the future. But the code is quite ugly and indeed relies on reflection... which maybe for Dictionary>>#at: is not that nice (e.g. if you want to create a minimal image without reflection) But it is save to add this to your own image, it works in Pharo11 and Pharo12 and I guess even in Pharo10 (not tested there). Marcus

May 19, 2023

This week (19/2023) on the Pharo Issue Tracker

by Marcus Denker

We released Pharo11 this week! see https://www.pharo.org/news/pharo11-released.html # Pharo 11 - Fix welcomebrowser version #13691 https://github.com/pharo-project/pharo/pull/13691 - Backport fixe #13663: Enabling the setting âShow lines numbers for all browsers #13665 https://github.com/pharo-project/pharo/pull/13665 - fix iceberg version for Pharo 11 #13655 https://github.com/pharo-project/pharo/pull/13655 - fix a bug with cmd+g usage #517 https://github.com/pharo-spec/NewTools/pull/517 - change spotter selection paste default #519 https://github.com/pharo-spec/NewTools/pull/519 # Pharo 12 ## Fixes - Improve some error logging #13689 https://github.com/pharo-project/pharo/pull/13689 - Enabling the setting âShow lines numbers for all browsersâ only shows line numbers for the first method selected in a class #13663 https://github.com/pharo-project/pharo/issues/13663 ## Variables - Allow-Slots-Shadow-PseudoVariables #13704 https://github.com/pharo-project/pharo/pull/13704 - Simplify ReservedVariable: #styleNameIn and remove #nameIsReserved: #13701 https://github.com/pharo-project/pharo/pull/13701 - ReservedVariable: No need for Singleton #13688 https://github.com/pharo-project/pharo/pull/13688 - Added thisProcess pseudovariable using existing bytecode. #13661 https://github.com/pharo-project/pharo/pull/13661 ## SystemOrganizer / RPackage - Remove dead code in RPackageOrganizer default instance #13660 https://github.com/pharo-project/pharo/pull/13660 - Reduces the references to RPackageOrganizer class>>default #13666 https://github.com/pharo-project/pharo/pull/13666 - Move system organizer to RPackage and remove references #13677 https://github.com/pharo-project/pharo/pull/13677 - Remove SystemOrganizer #13690 https://github.com/pharo-project/pharo/pull/13690 - Use Smalltalk organization in DependencyAnalyzer #13671 https://github.com/pharo-project/pharo/pull/13671 ## Protocols / Categories - Do not classify silently methods #13650 https://github.com/pharo-project/pharo/pull/13650 - Start protocol announcements testing + add feature to Announcer #13649 https://github.com/pharo-project/pharo/pull/13649 - Deprecate ProtocolRenamed #13659 https://github.com/pharo-project/pharo/pull/13659 - Small cleanup on protocol cleaning #13642 https://github.com/pharo-project/pharo/pull/13642 - Rename Protocol class>>#name: into #named: #13670 https://github.com/pharo-project/pharo/pull/13670 - Use real protocol in recategorization mecanism #13669 https://github.com/pharo-project/pharo/pull/13669 - Introduce ClassDescription>>#protocolNames and start using it #13658 https://github.com/pharo-project/pharo/pull/13658 - Add some tests on protocol announcements #13706 https://github.com/pharo-project/pharo/pull/13706 - Removal of nonexisting protocol should not announce anything #13702 https://github.com/pharo-project/pharo/pull/13702 ## Code Completion - Completion autoclose menu #13647 https://github.com/pharo-project/pharo/pull/13647 - Completion: improve completion on super #13679 https://github.com/pharo-project/pharo/pull/13679 - Completion: replacement put caret at the right place #13683 https://github.com/pharo-project/pharo/pull/13683 - Completion mini fixes #13696 https://github.com/pharo-project/pharo/pull/13696 - Basic type analysis for better completion #13687 https://github.com/pharo-project/pharo/pull/13687 - More type analysis #13697 https://github.com/pharo-project/pharo/pull/13697 - CompletionEngine do not automatically open empty completion menus #13705 https://github.com/pharo-project/pharo/pull/13705 - NECPreferences remove leftover of captureNavigationKeys #13703 https://github.com/pharo-project/pharo/pull/13703 - Completion: better update #13700 https://github.com/pharo-project/pharo/pull/13700 - Completion browse with Ctrl-B #13699 https://github.com/pharo-project/pharo/pull/13699 ## Cleanups - Clean some code using better condition branches #13668 https://github.com/pharo-project/pharo/pull/13668 - Remove some duplication in image cleaner #13676 https://github.com/pharo-project/pharo/pull/13676 - Message: rename ivar "args" to "arguments #13654 https://github.com/pharo-project/pharo/pull/13654 - Cleanup: Metacello-Platform #13682 https://github.com/pharo-project/pharo/pull/13682 - remove MetacelloPharo30Platform from 01-loadMetacello.st #13684 https://github.com/pharo-project/pharo/pull/13684 - Clean monticello class definition management #13680 https://github.com/pharo-project/pharo/pull/13680

May 15, 2023