Read-only oldspace

> Hi,
>
> If you read the squeak vm mailing they talk about read only objects.
> But wait, we've read only why should we change it?? In GST it's done by
> testing at "each" access if the object is RO or not. But we can do it lazily
> by using the hardware and operating system. Objects can be putted in a
> separate space with read-only access, thus we get a page fault when you try to
> change an object.
>
>

>  gst_object
> +_gst_alloc_ro_obj (size_t size,
> +                    OOP *p_oop)
> +{
> +  gst_object p_instance;
> +
> +  size = ROUNDED_BYTES (size);
> +
> +  /* If the object is big enough, we put it directly in oldspace.  */
> +  p_instance = (gst_object) _gst_mem_alloc (_gst_mem.ro_old, size);
> +  if COMMON (p_instance)
> +    goto ok;
> +
> +  _gst_global_gc (size);
> +  p_instance = (gst_object) _gst_mem_alloc (_gst_mem.ro_old, size);
> +  if COMMON (p_instance)
> +    goto ok;
> +
> +  _gst_compact (0);
> +  p_instance = (gst_object) _gst_mem_alloc (_gst_mem.ro_old, size);

> @@ -889,25 +1054,43 @@ oldspace_before_freeing (heap_data *h, heap_block *blk, size_t sz)
>  }
>
>  heap_data *
> -oldspace_nomemory (heap_data *h, size_t sz)
> +grow_nomemory (heap_data *h, size_t sz)
>  {
>    if (!_gst_gc_running)
>      _gst_global_gc (sz);
>    else
>      {
>        /* Already garbage collecting, emergency growth just to satisfy
> - tenuring necessities.  */
> -      int grow_amount_to_satisfy_rate = _gst_mem.old->heap_limit
> +         tenuring necessities.  */
> +      int grow_amount_to_satisfy_rate = h->heap_limit
>             * (100.0 + _gst_mem.space_grow_rate) / 100;
> -      int grow_amount_to_satisfy_threshold =
> -   (sz + _gst_mem.old->heap_total)
> -   * 100.0 /_gst_mem.grow_threshold_percent;
> +      int grow_amount_to_satisfy_threshold =
> +           (sz + h->heap_total)
> +           * 100.0 /_gst_mem.grow_threshold_percent;
>
> -      _gst_mem.old->heap_limit = MAX (grow_amount_to_satisfy_rate,
> -      grow_amount_to_satisfy_threshold);
> +      h->heap_limit = MAX (grow_amount_to_satisfy_rate,
> +                           grow_amount_to_satisfy_threshold);
>      }
>
> -  return _gst_mem.old;
> +  return h;

>> Paolo can you check the patch, here I've changed a bit the behavior of
>> make_ro; I preserve F_FIXED, F_OLD, or young information thus when
>> making an object writable I can restore it in the good space.
>> It seems to be the definitive version
>
> Just a few comments...
>
>> + /* become writeable */
>> + if ((oop->flags & F_LOADED) == 0)
>> + {
>> + if (oop->flags & F_FIXED)
>> + newObj = (gst_object) _gst_mem_alloc (_gst_mem.fixed, size);
>> + else if (oop->flags & F_OLD)
>> + newObj = (gst_object) _gst_mem_alloc (_gst_mem.old, size);
>> + else
>> + newObj = (gst_object) _gst_alloc_new_obj (size);
>
> This can use _gst_alloc_obj.
>
>> + if (oop->flags & F_READONLY)
>> + {
>> + oop->flags |= F_FIXED;
>
> Add a comment somewhere that rospace is never compacted. As a
> consequence...
>
>> gst_object
>> +_gst_alloc_ro_obj (size_t size,
>> + OOP *p_oop)
>> +{
>> + gst_object p_instance;
>> +
>> + size = ROUNDED_BYTES (size);
>> +
>> + /* If the object is big enough, we put it directly in oldspace. */
>> + p_instance = (gst_object) _gst_mem_alloc (_gst_mem.ro_old, size);
>> + if COMMON (p_instance)
>> + goto ok;
>> +
>> + _gst_global_gc (size);
>> + p_instance = (gst_object) _gst_mem_alloc (_gst_mem.ro_old, size);
>> + if COMMON (p_instance)
>> + goto ok;
>> +
>> + _gst_compact (0);
>> + p_instance = (gst_object) _gst_mem_alloc (_gst_mem.ro_old, size);
>
> ... the compaction step here is not necessary.
>
>> @@ -889,25 +1054,43 @@ oldspace_before_freeing (heap_data *h,
>> heap_block *blk, size_t sz)
>> }
>>
>> heap_data *
>> -oldspace_nomemory (heap_data *h, size_t sz)
>> +grow_nomemory (heap_data *h, size_t sz)
>> {
>> if (!_gst_gc_running)
>> _gst_global_gc (sz);
>> else
>> {
>> /* Already garbage collecting, emergency growth just to satisfy
>> - tenuring necessities. */
>> - int grow_amount_to_satisfy_rate = _gst_mem.old->heap_limit
>> + tenuring necessities. */
>> + int grow_amount_to_satisfy_rate = h->heap_limit
>> * (100.0 + _gst_mem.space_grow_rate) / 100;
>> - int grow_amount_to_satisfy_threshold =
>> - (sz + _gst_mem.old->heap_total)
>> - * 100.0 /_gst_mem.grow_threshold_percent;
>> + int grow_amount_to_satisfy_threshold =
>> + (sz + h->heap_total)
>> + * 100.0 /_gst_mem.grow_threshold_percent;
>>
>> - _gst_mem.old->heap_limit = MAX (grow_amount_to_satisfy_rate,
>> - grow_amount_to_satisfy_threshold);
>> + h->heap_limit = MAX (grow_amount_to_satisfy_rate,
>> + grow_amount_to_satisfy_threshold);
>> }
>>
>> - return _gst_mem.old;
>> + return h;
>
> This change to the "return" is wrong -- yes, it's tricky, I also
> misremembered it the other day and probably didn't explain it well
> enough on IRC. _gst_mem.old could have changed due to the GC. So just
> make this function void...
>
>> +}
>> +
>> +heap_data *
>> +ro_oldspace_nomemory (heap_data *h, size_t sz)
>> +{
>> + return grow_nomemory (_gst_mem.ro_old, sz);
>
> ... and return the appropriate space from these functions
> (_gst_mem.ro_old here and likewise elsewhere).
>
>> + if (flags & F_READONLY)
>> + object = (gst_object) _gst_mem_alloc (_gst_mem.ro_old, size);
>> + else if (flags & F_FIXED)
>
> This is bad unfortunately. There are a _lot_ of readonly objects, and
> allocating them will cause a measurable slowdown of image load. I'm not
> sure how you can fix that, perhaps by first storing readonly objects,
> then page aligning the file, and then storing other objects. Remember
> that any slowdown incurred by image save is acceptable.
>
> Otherwise looks good! Have you measured a practical performance
> difference with it?
>
> Paolo

> On 05/17/2011 10:39 PM, Gwenael Casaccio wrote:
>>>>
>>>
>>> This is bad unfortunately. There are a _lot_ of readonly objects, and
>>> allocating them will cause a measurable slowdown of image load. I'm not
>>> sure how you can fix that, perhaps by first storing readonly objects,
>>> then page aligning the file, and then storing other objects. Remember
>>> that any slowdown incurred by image save is acceptable.
>>
>> Paolo could I postpone the save issue to a third patch ?
>
> You can commit this patch to your repo and start working on it. :)
>
> Paolo

> On 05/25/2011 11:31 AM, Gwenael Casaccio wrote:
>>
>> Is it what you want for the image saving ?
>>
>> https://github.com/MrGwen/GNU-Smalltalk/commit/0372cfe08977da6d1d5cb8cecf2d7796f5375593
>>
>
> Yes! Just:
>
> 1) no cut-and-paste :)
>
> 2) if "use_copy_on_write" is true, you should mmap the read-only data to
> a separate memory region, so that you can make it read-only too. Perhaps
> you need to invoke mmap separately instead of doing it just once at the
> beginning of the loading.
>
> Paolo