Hi Marcel, all,

Documenting code - intentions, implementation details and tradeoffs etc. is a huge topic for me :)))

In mainstream languages is easy to find an answer to almost anything at stackoverflow etc. but in Squeak, given the lack of both documentation and millions of users, the in-image comments (in methods, classes) are, at least for me, crucial.

A special chapter would be documenting **changes** to the existing code: should we document reasons for each change? Was it a bugfix or a new feature, was it related to changes in other methods? Which tests address the change/bug? Are there some motivation examples?

A nice example would be `Context>>contextEnsure:`. A bug was found, `ctxt push: nil.` line was added to fix the bug plus a detailed note including a forum link. Perfect :) Now there's another bug in the method and I'd like to continue documenting the bug in a similar way - see Kernel-jar.1553 in the Inbox.

At the same time, in my opinion, the initial comment about the intentions and peculiarities of #contextEnsure: is insufficient (which is still an understatemnt). A good comment with examples would save hours of reverse engineering and guesswork (like the comment describing reasons for adding the `ctxt push: nil.` line) . (This is not to blame anyone, it's a 20+ years old code and I'm so grateful for all the effort that made Squeak alive and kicking! It's to say that for newcomers more detailed comments would be so much helpful.)

But back to the low space watcher. I didn't intend to study or "fix" the low space watcher but I accidentally observed some troubling behavior that seemed associated with the low space watcher and it was hard to learn what it does, how and why (without prior knowledge in this area). This is especially helpful during troubleshooting which, I'm sure, you all know too well :)

However, the big question is **where** to place such info. My opinion is **in the image**, as close to the code as possible, ideally inside the methods or class comments. In case of the low space watcher, however, in which method when the code is scattered across multiple methods? I can see some useful info in #lowSpaceThreshold but the first method you come into contact with is #installLowSpaceWatcher and #lowSpaceWatcher which don't say much.

How about placing more detailed or summarizing information in more complex cases like the low space watcher (with no class "LowSpaceWatcher" to hold the "central" description) to "documentation" class methods and just referencing them in the code comments (like "see more in SmalltalkImage>>docLowSpaceWatcher").

I also considered Squeak Help but writing Help requires extra effort; writing good comments is already very taxing so I wouldn't make it even more difficult :)

I'm afraid you didn't expect such a long text... sorry ;/

Thanks,
Jaromir


On 26-Jan-24 9:32:52 AM, "Taeumel, Marcel via Squeak-dev" <squeak-dev@lists.squeakfoundation.org> wrote:

Hi Jaromir --

What would be a good place to document the current state and intentions about low-space watcher? I mean, what would have helped you in this case?

Best,
Marcel

Am 25.01.2024 21:26:04 schrieb Jaromir Matas <mail@jaromir.net>:

Hi Marcel,

On 25-Jan-24 10:28:07 AM, "Taeumel, Marcel via Squeak-dev" <squeak-dev@lists.squeakfoundation.org> wrote:

Hi Jaromir --

> If you ask about the mechanism of the doubling behavior, I attached the explanation to the changeset

Yes, you explained why a specific piece of code causes complications. Still, I want to understand, why this piece of code is relevant.
Hmm, I guess you're right my example is (most likely) not relevant in the current image. I can't replicate the behavior in a practical situation. May I ask you to remove System-jar.1367 from the Inbox then? Sorry for the noise :)
Thanks!
Jaromir



For example, I could present some Morphic code, executed in multiple processes, that would then cause issues only to argue that, for example, there should be more semaphores in Collections or Morphic routines. Yet, there are design choices and contracts we should adhere to. Also, to avoid unnecessary code. Here, Morphic itself is not thread-safe and that is okay. It's by design. 

One should use #addDeferredUIMessage:, for example. :-)

Best,
Marcel

Am 25.01.2024 10:16:26 schrieb Jaromir Matas <mail@jaromir.net>:

Hi Marcel,

> How can this happen?

good question... I remember it (doubling the low space watcher process) bit me when I was experimenting with termination, I thought it was caused by a bug in my code but it turned out it was there before too :) Unfortunately I can't recall/find the exact situations. So I just posted it to make you aware.

> Would you elaborate on why this can happen at all?

If you ask about the mechanism of the doubling behavior, I attached the explanation to the changeset (System-jar.1367):

"Workspce example:
    p1 := [Smalltalk installLowSpaceWatcher] fork. 
    p2 := [Smalltalk installLowSpaceWatcher] fork
Here both proceses are scheduled in the run queue, then p1 wakes up, starts the LowSpaceProcess  
termination and waits on a semaphore until the termination is finished. Before the LowSpaceProcess
can proceed, process p2 wakes up and starts the LowSpaceProcess termination just like p1 and waits
on a semaphore until the termination is finished. Finally the LowSpaceProcess wakes up, unwinds, 
unblocks p1 a p2 and terminates. p1 now continues and installs a new LowSpaceProcess and then 
p2 does the same. The result will be two processes running the #lowSpaceWatcher method."


best,
Jaromir


On 25-Jan-24 8:44:35 AM, "Taeumel, Marcel via Squeak-dev" <squeak-dev@lists.squeakfoundation.org> wrote:

Hi Jaromir --

How can this happen? The only invocations of #installLowSpaceWatcher are in

Debugger >> proceed
Debugger >> windowIsClosing

Both are per definition not thread-safe (because GUI/Tool related) and must never be called from arbitrary processes.

Would you elaborate on why this can happen at all?
p1 := [Smalltalk installLowSpaceWatcher] fork.
p2 := [Smalltalk installLowSpaceWatcher] fork

Best,
Marcel

Am 25.01.2024 00:21:43 schrieb Jaromir Matas <mail@jaromir.net>:

Hi Eliot, hi all,

just in case: speaking of the LowSpaceWatcher, there's a bug in case of concurrent invocations of the LowSpaceWatcher which causes multiple LowSpaceWatchers running at the same time... if you try in the workspace:

p1 := [Smalltalk installLowSpaceWatcher] fork.
p2 := [Smalltalk installLowSpaceWatcher] fork

My suggestion is in System-jar.1367 (please disregard my note in the text about a bug in terminate, it's no longer relevant).

best,
Jaromir



On 24-Jan-24 8:06:45 PM, "Eliot Miranda" <eliot.miranda@gmail.com> wrote:

Hi Marcel, Hi All,

On Fri, Jan 19, 2024 at 6:33 AM Taeumel, Marcel via Squeak-dev <squeak-dev@lists.squeakfoundation.org> wrote:
Hi Eliot --

Hmm... our CI can successfully update a 22095 to HEAD without issues. Once per day. Hmm... I am not sure why the Debugger would be involved in a regular update. The debugger cue ist just for invocation. Even existing debugger windows should keep on working.

The reason my image locks up is that I'm playing with the low space notification and consequently the low space notifier is popping up frequently and I have to hit proceed.  Some day I'll revise the low space code appropriately; it's on my list.  But for the moment it shows that there's an update map missing.

When I load Tools-ct.1244 before updating, with the image starting at Tools-ct.1239, I am able to update.


What's the situation where the debugger wants to appear during an update?

Frequent low space notifications.


Best,
Marcel

Cheers!

Am 18.01.2024 23:36:44 schrieb Eliot Miranda <eliot.miranda@gmail.com>:

Hi All,

    I'm guessing there's some package ordering update missing to ensure the new debugger cue is defined.  I just attempted to update a VMMaker image and got the following infinite recursion:

     0x1465199e8 s UndefinedObject>handleSignal: 0x1186898e0: a(n) UndefinedObject
     0x146518030 s MessageNotUnderstood(Exception)>signal 0x146517c30: a(n) MessageNotUnderstood
     0x146517818 s UndefinedObject(Object)>doesNotUnderstand: new 0x1186898e0: a(n) UndefinedObject
     0x146517c78 s ProcessorScheduler>debugContext:title:full:contents: 0x11869afe0: a(n) ProcessorScheduler
     0x1465180e8 s ECToolSet class(StandardToolSet class)>handleError: 0x1195d6c88: a(n) ECToolSet
     0x146518500 s ToolSet class>handleError: 0x118a26690: a(n) ToolSet
     0x146518900 s UnhandledError>defaultAction 0x146518e50: a(n) UnhandledError
     0x146518d98 s UndefinedObject>handleSignal: 0x1186898e0: a(n) UndefinedObject
     0x1465191d0 s UnhandledError(Exception)>signal 0x146518e50: a(n) UnhandledError
     0x146519668 s UnhandledError class>signalForException: 0x118a27df8: a(n) UnhandledError
     0x146519aa0 s MessageNotUnderstood(Error)>defaultAction 0x1465185b8: a(n) MessageNotUnderstood
     0x146519f38 s MessageNotUnderstood>defaultAction 0x1465185b8: a(n) MessageNotUnderstood
     0x14651a370 s UndefinedObject>handleSignal: 0x1186898e0: a(n) UndefinedObject
     0x1465189b8 s MessageNotUnderstood(Exception)>signal 0x1465185b8: a(n) MessageNotUnderstood
     0x1465181a0 s UndefinedObject(Object)>doesNotUnderstand: new 0x1186898e0: a(n) UndefinedObject
     0x146518600 s ProcessorScheduler>debugContext:title:full:contents: 0x11869afe0: a(n) ProcessorScheduler
     0x146518a70 s ECToolSet class(StandardToolSet class)>handleError: 0x1195d6c88: a(n) ECToolSet
     0x146518e88 s ToolSet class>handleError: 0x118a26690: a(n) ToolSet
     0x146519288 s UnhandledError>defaultAction 0x1465197d8: a(n) UnhandledError
     0x146519720 s UndefinedObject>handleSignal: 0x1186898e0: a(n) UndefinedObject
     0x146519b58 s UnhandledError(Exception)>signal 0x1465197d8: a(n) UnhandledError
     0x146519ff0 s UnhandledError class>signalForException: 0x118a27df8: a(n) UnhandledError
     0x14651a428 s MessageNotUnderstood(Error)>defaultAction 0x146518f40: a(n) MessageNotUnderstood
     0x14651a8c0 s MessageNotUnderstood>defaultAction 0x146518f40: a(n) MessageNotUnderstood
     0x14651acf8 s UndefinedObject>handleSignal: 0x1186898e0: a(n) UndefinedObject
     0x146519340 s MessageNotUnderstood(Exception)>signal 0x146518f40: a(n) MessageNotUnderstood
     0x146518b28 s UndefinedObject(Object)>doesNotUnderstand: new 0x1186898e0: a(n) UndefinedObject
     0x146518f88 s ProcessorScheduler>debugContext:title:full:contents: 0x11869afe0: a(n) ProcessorScheduler
     0x1465193f8 s ECToolSet class(StandardToolSet class)>handleError: 0x1195d6c88: a(n) ECToolSet
     0x146519810 s ToolSet class>handleError: 0x118a26690: a(n) ToolSet
     0x146519c10 s UnhandledError>defaultAction 0x14651a160: a(n) UnhandledError
     0x14651a0a8 s UndefinedObject>handleSignal: 0x1186898e0: a(n) UndefinedObject
     0x14651a4e0 s UnhandledError(Exception)>signal 0x14651a160: a(n) UnhandledError
     0x14651a978 s UnhandledError class>signalForException: 0x118a27df8: a(n) UnhandledError
     0x14651adb0 s MessageNotUnderstood(Error)>defaultAction 0x1465198c8: a(n) MessageNotUnderstood
     0x14651b248 s MessageNotUnderstood>defaultAction 0x1465198c8: a(n) MessageNotUnderstood
     0x14651b680 s UndefinedObject>handleSignal: 0x1186898e0: a(n) UndefinedObject
     0x146519cc8 s MessageNotUnderstood(Exception)>signal 0x1465198c8: a(n) MessageNotUnderstood
     0x1465194b0 s UndefinedObject(Object)>doesNotUnderstand: new 0x1186898e0: a(n) UndefinedObject
     0x146519910 s ProcessorScheduler>debugContext:title:full:contents: 0x11869afe0: a(n) ProcessorScheduler
_,,,^..^,,,_
best, Eliot



--
_,,,^..^,,,_
best, Eliot