Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding option for pin behaviour #844

Merged

Conversation

tesonep
Copy link
Collaborator

@tesonep tesonep commented Aug 26, 2024

When pinning young objects, the objects are clonned into the old space.
It tries to allocate the object in a segment with already pinned objects.
Avoid the clonning process avoid this search and allocate the clonned object anywhere?

Copy link
Contributor

@akgrant43 akgrant43 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the comments in SpurMemoryManager>>pinObject: need to be updated to reflect the modified behaviour, e.g.:

pinObject: objOop
    "Attempt to pin objOop, which must not be immediate.
     If the attempt succeeds answer objOop's (possibly moved) oop.
     If the attempt fails, which can only occur if there is no memory, answer 0."
    <inline: false>
    | oldClone seg |
    <var: #seg type: #'SpurSegmentInfo *'>
    self assert: (self isNonImmediate: objOop).
    "If the object is already old and is:
    - large, or
    - already in a old space segment containing other pinned objects, or
    - no other old space segment contains pinned objects
    then just pin the object."
    (memoryMap isOldObject: objOop) ifTrue:
        [(self numBytesOf: objOop) > (1024 * 1024) ifTrue:
            [self setIsPinnedOf: objOop to: true.
             ^objOop].
         seg := segmentManager segmentContainingObj: objOop.
         seg containsPinned ifTrue:
            [self setIsPinnedOf: objOop to: true.
             ^objOop].
         segmentManager someSegmentContainsPinned ifFalse:
            [self setIsPinnedOf: objOop to: true.
             seg containsPinned: true.
             ^objOop]].
    "Otherwise ensure the object is in old space and defer the decision whether to group pinned objects together"
    oldClone := self cloneInOldSpace: objOop forPinning: true.
    oldClone ~= 0 ifTrue:
        [becomeEffectsFlags := self becomeEffectFlagsFor: objOop.
         self setIsPinnedOf: oldClone to: true.
         self forward: objOop to: oldClone.
         self followSpecialObjectsOop.
         coInterpreter postBecomeAction: becomeEffectsFlags.
         self postBecomeScanClassTable: becomeEffectsFlags.
         becomeEffectsFlags := 0].
    ^oldClone 
    ```

@tesonep tesonep merged commit e71192e into pharo-project:pharo-10 Aug 30, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants