[Pharo-project] On cleaning image startup list
Hi, i am checking image startup list trying to see if anything uses DateAndTime now (or anything with "now") before DateAndTime does own "startup".. which i suspecting causing this hung process spawning. Due to brittle logic in DateAndTime startup, i think we should promote it closer to Delay startup.. because right now it looks like following: an OrderedCollection(#Delay #OSPlatform #DiskStore #DisplayScreen #Cursor #InputEventFetcher #InputEventSensor #ProcessorScheduler #NaturalLanguageTranslator #ShortIntegerArray #PasteUpMorph #SmalltalkImage #CPUWatcher #Clipboard #InternetConfiguration #Locale #MultiByteFileStream #UUIDGenerator #WeakArray #MacOSClipboard #RecentMessageList #EditorFindReplaceDialogWindow #ZnServer #DateAndTime #FreeTypeSettings #LanguageEnvironment #FileStream #FileLocator #StartupLoader #DefaultCommandLineHandler #FT2Handle) Also, along the way, i found some stuff which to me looks like abuse: NaturalLanguageTranslator, RecentMessageList - already reported on bugs tracker The EditorFindReplaceDialogWindow this one is strange too.. and i'd like to ask about. startUp "This message is sent to registered classes when the system is coming up." Singleton ifNotNil: [Singleton close. Singleton := nil] may i ask why it is sooo important for system to keep some arbitrary morph closed when image goes up? I do not see a valid reason for this, but i might be miss something.. IMO if something can be solved without adding things to startup, it should be solved like that. In general, I think it is logical that everything which can potentially uses dates (like ZnServer, FileSystem & al) should start up after DateAndTime, even if they are not using dates in their own startup method. (but they could spawn threads which do) -- Best regards, Igor Stasenko.
Another strange thing, which i discovered is deferredStartupActions, is not empty in my image.. (you can check it by inspecting Smalltalk variable) an OrderedCollection([self managedServers do: [:each | each start]]) Sven, please check this. Maybe it is my image.. -- Best regards, Igor Stasenko.
On 14 Apr 2013, at 20:24, Igor Stasenko <siguctua@gmail.com> wrote:
Another strange thing, which i discovered is deferredStartupActions, is not empty in my image..
(you can check it by inspecting Smalltalk variable)
an OrderedCollection([self managedServers do: [:each | each start]])
Sven, please check this. Maybe it is my image..
Euh, yes, that is what ZnServer class>>#startUp: does. There is even a comment. I don't remember why it is done this way, I guess someone suggested it to me. Is there anything wrong with that ?
-- Best regards, Igor Stasenko.
-- Sven Van Caekenberghe http://stfx.eu Smalltalk is the Red Pill
On 14 April 2013 20:33, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 14 Apr 2013, at 20:24, Igor Stasenko <siguctua@gmail.com> wrote:
Another strange thing, which i discovered is deferredStartupActions, is not empty in my image..
(you can check it by inspecting Smalltalk variable)
an OrderedCollection([self managedServers do: [:each | each start]])
Sven, please check this. Maybe it is my image..
Euh, yes, that is what ZnServer class>>#startUp: does. There is even a comment. I don't remember why it is done this way, I guess someone suggested it to me. Is there anything wrong with that ?
yes. the code by itself looks quite harmless. but i don't understand why deferredStartupActions list is not empty (it should be empty, and stay empty once image finished startup)
-- Best regards, Igor Stasenko.
-- Sven Van Caekenberghe http://stfx.eu Smalltalk is the Red Pill
-- Best regards, Igor Stasenko.
On 14 Apr 2013, at 20:36, Igor Stasenko <siguctua@gmail.com> wrote:
On 14 April 2013 20:33, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 14 Apr 2013, at 20:24, Igor Stasenko <siguctua@gmail.com> wrote:
Another strange thing, which i discovered is deferredStartupActions, is not empty in my image..
(you can check it by inspecting Smalltalk variable)
an OrderedCollection([self managedServers do: [:each | each start]])
Sven, please check this. Maybe it is my image..
Euh, yes, that is what ZnServer class>>#startUp: does. There is even a comment. I don't remember why it is done this way, I guess someone suggested it to me. Is there anything wrong with that ?
yes. the code by itself looks quite harmless. but i don't understand why deferredStartupActions list is not empty (it should be empty, and stay empty once image finished startup)
OK, you mean that once a deferred action is executed, it should be removed from the list. If the list is not empty, that seems like an internal problem.
-- Best regards, Igor Stasenko.
-- Sven Van Caekenberghe http://stfx.eu Smalltalk is the Red Pill
-- Best regards, Igor Stasenko.
On 14 April 2013 20:41, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 14 Apr 2013, at 20:36, Igor Stasenko <siguctua@gmail.com> wrote:
On 14 April 2013 20:33, Sven Van Caekenberghe <sven@stfx.eu> wrote:
On 14 Apr 2013, at 20:24, Igor Stasenko <siguctua@gmail.com> wrote:
Another strange thing, which i discovered is deferredStartupActions, is not empty in my image..
(you can check it by inspecting Smalltalk variable)
an OrderedCollection([self managedServers do: [:each | each start]])
Sven, please check this. Maybe it is my image..
Euh, yes, that is what ZnServer class>>#startUp: does. There is even a comment. I don't remember why it is done this way, I guess someone suggested it to me. Is there anything wrong with that ?
yes. the code by itself looks quite harmless. but i don't understand why deferredStartupActions list is not empty (it should be empty, and stay empty once image finished startup)
OK, you mean that once a deferred action is executed, it should be removed from the list. If the list is not empty, that seems like an internal problem.
it seems so.. because nothing cleaning it after.. in #executeDeferredStartupActions: (https://pharo.fogbugz.com/default.asp?10287) -- Best regards, Igor Stasenko.
On topic: i did not succeeded in finding who is the first guy calling 'DateAndTime now' and so i going to patch the code to detect it. -- Best regards, Igor Stasenko.
pfff... DateAndTime class>>initialize super initialize. NEVER EVER call super initialize for class initializers! I think we should add a lint rule for that. -- Best regards, Igor Stasenko.
On 14 April 2013 21:27, Igor Stasenko <siguctua@gmail.com> wrote:
pfff...
DateAndTime class>>initialize
super initialize.
NEVER EVER call super initialize for class initializers!
I think we should add a lint rule for that.
(https://pharo.fogbugz.com/default.asp?10288)
-- Best regards, Igor Stasenko.
-- Best regards, Igor Stasenko.
the more i learn about DateAndTime the more i lean towards completely rewriting this ... err.. crap. (yes. sorry for harsh word). DateAndTime class>>now is not thread-safe.. now | nanoTicks msm | nanoTicks := (msm := self milliSecondsSinceMidnight) * 1000000. (LastTick < nanoTicks) ifTrue: [ LastTick := nanoTicks. ^ self todayAtMilliSeconds: msm]. LastTickSemaphore critical: [ LastTick := LastTick + 1. ^ self todayAtNanoSeconds: LastTick] Wrapping only single assignment to variable with critical section while leaving another without it is same as if not using critical section at all. -- Best regards, Igor Stasenko.
I was wondering about that method the other day... Now, you made it clear: crap! 2013/4/14 Igor Stasenko <siguctua@gmail.com>
the more i learn about DateAndTime the more i lean towards completely rewriting this ... err.. crap. (yes. sorry for harsh word).
DateAndTime class>>now is not thread-safe..
now | nanoTicks msm |
nanoTicks := (msm := self milliSecondsSinceMidnight) * 1000000.
(LastTick < nanoTicks) ifTrue: [ LastTick := nanoTicks. ^ self todayAtMilliSeconds: msm].
LastTickSemaphore critical: [ LastTick := LastTick + 1. ^ self todayAtNanoSeconds: LastTick]
Wrapping only single assignment to variable with critical section while leaving another without it is same as if not using critical section at all.
-- Best regards, Igor Stasenko.
Please rewrite it :) Stef On Apr 14, 2013, at 9:52 PM, Igor Stasenko <siguctua@gmail.com> wrote:
the more i learn about DateAndTime the more i lean towards completely rewriting this ... err.. crap. (yes. sorry for harsh word).
DateAndTime class>>now is not thread-safe..
now | nanoTicks msm |
nanoTicks := (msm := self milliSecondsSinceMidnight) * 1000000.
(LastTick < nanoTicks) ifTrue: [ LastTick := nanoTicks. ^ self todayAtMilliSeconds: msm].
LastTickSemaphore critical: [ LastTick := LastTick + 1. ^ self todayAtNanoSeconds: LastTick]
Wrapping only single assignment to variable with critical section while leaving another without it is same as if not using critical section at all.
-- Best regards, Igor Stasenko.
I'll soon be in need of a high quality DateAndTime implementation. 2013/4/15 stephane ducasse <stephane.ducasse@free.fr>
Please rewrite it :)
Stef
On Apr 14, 2013, at 9:52 PM, Igor Stasenko <siguctua@gmail.com> wrote:
the more i learn about DateAndTime the more i lean towards completely rewriting this ... err.. crap. (yes. sorry for harsh word).
DateAndTime class>>now is not thread-safe..
now | nanoTicks msm |
nanoTicks := (msm := self milliSecondsSinceMidnight) * 1000000.
(LastTick < nanoTicks) ifTrue: [ LastTick := nanoTicks. ^ self todayAtMilliSeconds: msm].
LastTickSemaphore critical: [ LastTick := LastTick + 1. ^ self todayAtNanoSeconds: LastTick]
Wrapping only single assignment to variable with critical section while leaving another without it is same as if not using critical section at all.
-- Best regards, Igor Stasenko.
yes good idea! Stef On Apr 14, 2013, at 9:32 PM, Igor Stasenko <siguctua@gmail.com> wrote:
On 14 April 2013 21:27, Igor Stasenko <siguctua@gmail.com> wrote:
pfff...
DateAndTime class>>initialize
super initialize.
NEVER EVER call super initialize for class initializers!
I think we should add a lint rule for that.
(https://pharo.fogbugz.com/default.asp?10288)
-- Best regards, Igor Stasenko.
-- Best regards, Igor Stasenko.
On 15 April 2013 00:31, Norbert Hartl <norbert@hartl.name> wrote:
Am 14.04.2013 um 21:27 schrieb Igor Stasenko <siguctua@gmail.com>:
pfff...
DateAndTime class>>initialize
super initialize.
NEVER EVER call super initialize for class initializers!
I think we should add a lint rule for that.
Can you explain why?
because if you would strictly follow that rule, then you should end up in Behavior>>initialize and your class will be reset completely: initialize "moved here from the class side's #new" super initialize. superclass := Object. "no longer sending any messages, some of them crash the VM" methodDict := self emptyMethodDictionary. format := Object format subclasses sharing all class vars declared in supeclasses. resetting them every time you creating/initializing a subclass is just waste. It only makes sense when your superclass having own class inst var to initialize. But in most cases you want to initialize them differently than in superclass (e.g. superclass ivar of your superclass should be not same as your class).
Norbert
-- Best regards, Igor Stasenko.
Here's another measurement.. how often DateAndTime now drifts from system time. (put this method on DateAndTime class side) calculateDrift | durationSinceEpoch systemSeconds milSince secDrifted systemSecondsSinceMidnight | "those two must be close to minimize time distance between measurement" systemSeconds := Time primSecondsClock. " uncomment to see that drift calculation is correct (Delay forSeconds: 5) wait. " milSince := self milliSecondsSinceMidnight. secDrifted := milSince // 1000. durationSinceEpoch := Duration days: SqueakEpoch hours: 0 minutes: 0 seconds: systemSeconds. systemSecondsSinceMidnight := durationSinceEpoch - (durationSinceEpoch days days). ^(Duration seconds: secDrifted) - systemSecondsSinceMidnight ============ Now the experiment, we run this stuff in a tight loop and see how many times the drift (expressed in seconds is non-zero): | count | count := 0. 1 to: 1000000 do: [:i | DateAndTime calculateDrift isZero ifFalse: [ count := count +1 ] ]. count / 1000000 asFloat 0.00142 So, "statistically" every ~1000'th request. "DateAndTime now" is wrong by 1 second than system time.. -- Best regards, Igor Stasenko.
participants (5)
-
Igor Stasenko -
Norbert Hartl -
phil＠highoctane.be -
stephane ducasse -
Sven Van Caekenberghe