pharo icon indicating copy to clipboard operation
pharo copied to clipboard

Simplify `#instanceCount`

Open seandenigris opened this issue 3 years ago • 6 comments

Ordinarily I'd just do a PR, but since so low level kernel I wanted to make sure I wasn't missing something obvious first...

Before

The current implementation is:

instanceCount
	"Answer the number of instances of the receiver that are currently in 
	use."

	| count |
	count := 0.
	self allInstancesDo: [:x | count := count + 1].
	^count

where #allInstancesDo: uses <primitive: 177> to get the collection of instances upfront.

After

It seems it could simply be:

instanceCount
	"Answer the number of instances of the receiver that are currently in 
	use."

	^ self allInstances size

where #allInstances uses the same primitive

seandenigris avatar Mar 11 '22 17:03 seandenigris

I checked: yes, you are right! I think in the past, #allInstancesDo: was iterating using #nextObject, so using it was avoiding to allocate the bug array... but now we have a primitive that returns this array and even allInstancesDo: uses that.

MarcusDenker avatar May 20 '22 07:05 MarcusDenker

#instancesSizeInMemory is similar

MarcusDenker avatar May 20 '22 07:05 MarcusDenker

Looking at this again... #allInstancesDo: does have a special case when memory is too low to allocate the array:

"allInstancesOrNil can fail because memory is low.  If so, fall back on the old
enumeration code.  Because aBlock might change the class of inst (for example,
using become:), it is essential to compute next before aBlock value: inst."

It falls back on iterating without first creating the array:

inst := self someInstance.
	[inst == nil] whileFalse:
		[next := inst nextInstance.
		 aBlock value: inst.
		 inst := next]

if we use #allInstances directly, we would loose that fallback and in case of no memory we would fail.

MarcusDenker avatar Jun 15 '22 07:06 MarcusDenker

But it still leaves the original issue of #instanceCount sending #allInstances, no?

If so, it's simple enough that I should be able to get around to this soon...

seandenigris avatar Jun 15 '22 13:06 seandenigris

instanceCount uses allInstancesDo: so that if memory is not there to allocate the big array, it uses the fallback.

If instanceCount would use allInstances, it would just fail.

MarcusDenker avatar Jun 15 '22 14:06 MarcusDenker

allInstancesDo: aBlock
	"Evaluate aBlock with each of the current instances of the receiver."
	| instances inst next |
	instances := self allInstancesOrNil.
	instances ifNotNil:
		[instances do: aBlock.
		 ^self].
	"allInstancesOrNil can fail because memory is low.  If so, fall back on the old
	 enumeration code.  Because aBlock might change the class of inst (for example,
	 using become:), it is essential to compute next before aBlock value: inst."
	inst := self someInstance.
	[inst == nil] whileFalse:
		[next := inst nextInstance.
		 aBlock value: inst.
		 inst := next]

MarcusDenker avatar Jun 15 '22 14:06 MarcusDenker