[pharo-project/pharo-core] 6a5096: 40477
Branch: refs/heads/4.0 Home: https://github.com/pharo-project/pharo-core Commit: 6a509693b22a9920f2a59f5c5194e612c27af9fc https://github.com/pharo-project/pharo-core/commit/6a509693b22a9920f2a59f5c5... Author: Jenkins Build Server <board@pharo-project.org> Date: 2015-02-05 (Thu, 05 Feb 2015) Changed paths: M Fuel.package/FLCompiledMethodCluster.class/instance/serialize%2Fmaterialize/materializeInstanceWith_.st M Fuel.package/FLCompiledMethodCluster.class/instance/serialize%2Fmaterialize/serializeInstance_with_.st M Fuel.package/FLMaterializer.class/class/protected/currentVersion.st M Fuel.package/FLSerializer.class/class/protected/currentVersion.st A Kernel.package/Delay.class/instance/delaying/waitOtherwise_.st M Morphic-Core.package/WorldState.class/instance/update cycle/interCyclePause_.st A ScriptLoader40.package/ScriptLoader.class/instance/pharo - scripts/script477.st A ScriptLoader40.package/ScriptLoader.class/instance/pharo - updates/update40477.st M ScriptLoader40.package/ScriptLoader.class/instance/public/commentForCurrentUpdate.st Log Message: ----------- 40477 14688 Change CompiledMethod header serialization to use signed integer https://pharo.fogbugz.com/f/cases/14688 14669 Delay refactoring (part 2a) - avoid UI locking up when timer event loop is stopped https://pharo.fogbugz.com/f/cases/14669 http://files.pharo.org/image/40/40477.zip
On Thu, Feb 5, 2015 at 11:12 PM, GitHub <noreply@github.com> wrote:
Log Message: ----------- 40477
14669 Delay refactoring (part 2a) - avoid UI locking up when timer event loop is stopped https://pharo.fogbugz.com/f/cases/14669
Thanks Max and Marcus for review and pushing this through. Next step is Issue 14353 "Delay refactoring (Part 2) - change from milliseconds to microseconds" and I started to review rebasing this onto the latest build. Now left over from Part 1 ( https://pharo.fogbugz.com/default.asp?14261) is lots of conditional code wrap of the form... self newCodeEnabled ifTrue: [ "my new code" ] ifFalse: [ "the old code" ] However this conditional code wrap clutters things making it harder to review changes going forward. So I was wondering it was preferable for me to clean that out first, before pushing 14353 for review. For Part 1 it served a few purposes... (1) It was required to make an instantaneous transition to the new architecture. However this no longer required since integration of Issue 14669 now allows the DelayScheduler to be stopped to change code without locking the system. (2) It provided an emergency fallback to disable the new architecture, since this was deep-diving into critical infrastructure. However Part 1 was integrated in November, so is that long enough for the code to have proved itself? (3) It allowed easy switching back and forth for benchmarking. Maybe still useful, but I don't think this is enough on its own to keep the newCodeEnabled conditional code wraps. Benchmarking can still be done on separate build before and after integration of subsequent changes. Please advise how you'd like me to proceed. cheers -ben
On 05 Feb 2015, at 17:46, Ben Coman <btc@openInWorld.com> wrote:
On Thu, Feb 5, 2015 at 11:12 PM, GitHub <noreply@github.com <mailto:noreply@github.com>> wrote:
Log Message: ----------- 40477
14669 Delay refactoring (part 2a) - avoid UI locking up when timer event loop is stopped https://pharo.fogbugz.com/f/cases/14669 <https://pharo.fogbugz.com/f/cases/14669>
Thanks Max and Marcus for review and pushing this through.
Next step is Issue 14353 "Delay refactoring (Part 2) - change from milliseconds to microseconds" and I started to review rebasing this onto the latest build. Now left over from Part 1 (https://pharo.fogbugz.com/default.asp?14261 <https://pharo.fogbugz.com/default.asp?14261>) is lots of conditional code wrap of the form...
self newCodeEnabled ifTrue: [ "my new code" ] ifFalse: [ "the old code" ]
However this conditional code wrap clutters things making it harder to review changes going forward. So I was wondering it was preferable for me to clean that out first, before pushing 14353 for review. For Part 1 it served a few purposes...
(1) It was required to make an instantaneous transition to the new architecture. However this no longer required since integration of Issue 14669 now allows the DelayScheduler to be stopped to change code without locking the system.
(2) It provided an emergency fallback to disable the new architecture, since this was deep-diving into critical infrastructure. However Part 1 was integrated in November, so is that long enough for the code to have proved itself?
I would say so, yes.
(3) It allowed easy switching back and forth for benchmarking. Maybe still useful, but I don't think this is enough on its own to keep the newCodeEnabled conditional code wraps. Benchmarking can still be done on separate build before and after integration of subsequent changes.
You can recompile on the fly if you really need to use the same image.
Please advise how you'd like me to proceed. cheers -ben
Iâd go ahead and remove the conditionals and the old code. The release of Pharo 4 is planned for April I think, so even if something goes wrong, thereâs still time to fix it. Removing the clutter will also make it easier to review your changes; and it will make writing code for you easier. Pharo 4 is still in development (unless I missed something), and development means unstable environment. People who use Pharo 4 are aware of that. Thatâs my two cents anyway. Keep up the great work! Cheers, Max
On 05 Feb 2015, at 21:34, Max Leske <maxleske@gmail.com> wrote:
On 05 Feb 2015, at 17:46, Ben Coman <btc@openInWorld.com <mailto:btc@openInWorld.com>> wrote:
On Thu, Feb 5, 2015 at 11:12 PM, GitHub <noreply@github.com <mailto:noreply@github.com>> wrote:
Log Message: ----------- 40477
14669 Delay refactoring (part 2a) - avoid UI locking up when timer event loop is stopped https://pharo.fogbugz.com/f/cases/14669 <https://pharo.fogbugz.com/f/cases/14669>
Thanks Max and Marcus for review and pushing this through.
Next step is Issue 14353 "Delay refactoring (Part 2) - change from milliseconds to microseconds" and I started to review rebasing this onto the latest build. Now left over from Part 1 (https://pharo.fogbugz.com/default.asp?14261 <https://pharo.fogbugz.com/default.asp?14261>) is lots of conditional code wrap of the form...
self newCodeEnabled ifTrue: [ "my new code" ] ifFalse: [ "the old code" ]
However this conditional code wrap clutters things making it harder to review changes going forward. So I was wondering it was preferable for me to clean that out first, before pushing 14353 for review. For Part 1 it served a few purposes...
(1) It was required to make an instantaneous transition to the new architecture. However this no longer required since integration of Issue 14669 now allows the DelayScheduler to be stopped to change code without locking the system.
(2) It provided an emergency fallback to disable the new architecture, since this was deep-diving into critical infrastructure. However Part 1 was integrated in November, so is that long enough for the code to have proved itself?
I would say so, yes.
(3) It allowed easy switching back and forth for benchmarking. Maybe still useful, but I don't think this is enough on its own to keep the newCodeEnabled conditional code wraps. Benchmarking can still be done on separate build before and after integration of subsequent changes.
You can recompile on the fly if you really need to use the same image.
Please advise how you'd like me to proceed. cheers -ben
Iâd go ahead and remove the conditionals and the old code. The release of Pharo 4 is planned for April I think, so even if something goes wrong, thereâs still time to fix it. Removing the clutter will also make it easier to review your changes; and it will make writing code for you easier.
Yes! If you can choose between âcleaning upâ and âcontinue to workâ, choose cleaning up. It will be the right choice in most cases⦠Marcus
On 06 Feb 2015, at 09:07, Marcus Denker <marcus.denker@inria.fr> wrote:
Log Message: ----------- 40477
14669 Delay refactoring (part 2a) - avoid UI locking up when timer event loop is stopped https://pharo.fogbugz.com/f/cases/14669
Thanks Max and Marcus for review and pushing this through.
It seems to have some problem: WorldState>>#interCyclePause: failed https://pharo.fogbugz.com/f/cases/14874 When e.g. in Nautilus searching for a class. Marcus
I see it as well from time to time in #40478 but not often and it does not seem to have any negative effect, I can just keep on working and there is no obvious slowdown.
On 06 Feb 2015, at 16:23, Marcus Denker <marcus.denker@inria.fr> wrote:
On 06 Feb 2015, at 09:07, Marcus Denker <marcus.denker@inria.fr> wrote:
Log Message: ----------- 40477
14669 Delay refactoring (part 2a) - avoid UI locking up when timer event loop is stopped https://pharo.fogbugz.com/f/cases/14669
Thanks Max and Marcus for review and pushing this through.
It seems to have some problem:
WorldState>>#interCyclePause: failed https://pharo.fogbugz.com/f/cases/14874
When e.g. in Nautilus searching for a class.
Marcus
My first thought is that it may expose some other bug previously hidden, but that could just be bravado. It is late here, so I will take a look tomorrow. cheers -ben On Fri, Feb 6, 2015 at 11:24 PM, Sven Van Caekenberghe <sven@stfx.eu> wrote:
I see it as well from time to time in #40478 but not often and it does not seem to have any negative effect, I can just keep on working and there is no obvious slowdown.
On 06 Feb 2015, at 16:23, Marcus Denker <marcus.denker@inria.fr> wrote:
On 06 Feb 2015, at 09:07, Marcus Denker <marcus.denker@inria.fr> wrote:
Log Message: ----------- 40477
14669 Delay refactoring (part 2a) - avoid UI locking up when timer
event loop is stopped
https://pharo.fogbugz.com/f/cases/14669
Thanks Max and Marcus for review and pushing this through.
It seems to have some problem:
WorldState>>#interCyclePause: failed https://pharo.fogbugz.com/f/cases/14874
When e.g. in Nautilus searching for a class.
Marcus
Just for history. No hidden bug. It was an race condition introduced by my code. Fix has been integrated for this issue... https://pharo.fogbugz.com/default.asp?14874 cheers -ben On Sat, Feb 7, 2015 at 1:08 AM, Ben Coman <btc@openinworld.com> wrote:
My first thought is that it may expose some other bug previously hidden, but that could just be bravado. It is late here, so I will take a look tomorrow. cheers -ben
On Fri, Feb 6, 2015 at 11:24 PM, Sven Van Caekenberghe <sven@stfx.eu> wrote:
I see it as well from time to time in #40478 but not often and it does not seem to have any negative effect, I can just keep on working and there is no obvious slowdown.
On 06 Feb 2015, at 16:23, Marcus Denker <marcus.denker@inria.fr> wrote:
On 06 Feb 2015, at 09:07, Marcus Denker <marcus.denker@inria.fr> wrote:
Log Message: ----------- 40477
14669 Delay refactoring (part 2a) - avoid UI locking up when timer
event loop is stopped
https://pharo.fogbugz.com/f/cases/14669
Thanks Max and Marcus for review and pushing this through.
It seems to have some problem:
WorldState>>#interCyclePause: failed https://pharo.fogbugz.com/f/cases/14874
When e.g. in Nautilus searching for a class.
Marcus
+1 STef Le 5/2/15 21:34, Max Leske a écrit :
On 05 Feb 2015, at 17:46, Ben Coman <btc@openInWorld.com <mailto:btc@openInWorld.com>> wrote:
On Thu, Feb 5, 2015 at 11:12 PM, GitHub <noreply@github.com <mailto:noreply@github.com>> wrote:
Log Message: ----------- 40477
14669 Delay refactoring (part 2a) - avoid UI locking up when timer event loop is stopped https://pharo.fogbugz.com/f/cases/14669
Thanks Max and Marcus for review and pushing this through.
Next step is Issue 14353 "Delay refactoring (Part 2) - change from milliseconds to microseconds" and I started to review rebasing this onto the latest build. Now left over from Part 1 (https://pharo.fogbugz.com/default.asp?14261) is lots of conditional code wrap of the form...
self newCodeEnabled ifTrue: [ "my new code" ] ifFalse: [ "the old code" ]
However this conditional code wrap clutters things making it harder to review changes going forward. So I was wondering it was preferable for me to clean that out first, before pushing 14353 for review. For Part 1 it served a few purposes...
(1) It was required to make an instantaneous transition to the new architecture. However this no longer required since integration of Issue 14669 now allows the DelayScheduler to be stopped to change code without locking the system.
(2) It provided an emergency fallback to disable the new architecture, since this was deep-diving into critical infrastructure. However Part 1 was integrated in November, so is that long enough for the code to have proved itself?
I would say so, yes.
(3) It allowed easy switching back and forth for benchmarking. Maybe still useful, but I don't think this is enough on its own to keep the newCodeEnabled conditional code wraps. Benchmarking can still be done on separate build before and after integration of subsequent changes.
You can recompile on the fly if you really need to use the same image.
Please advise how you'd like me to proceed. cheers -ben
Iâd go ahead and remove the conditionals and the old code. The release of Pharo 4 is planned for April I think, so even if something goes wrong, thereâs still time to fix it. Removing the clutter will also make it easier to review your changes; and it will make writing code for you easier.
Pharo 4 is still in development (unless I missed something), and development means unstable environment. People who use Pharo 4 are aware of that.
Thatâs my two cents anyway.
Keep up the great work!
Cheers, Max
participants (6)
-
Ben Coman -
GitHub -
Marcus Denker -
Max Leske -
stepharo -
Sven Van Caekenberghe