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

Many gcs #653

Open
wants to merge 20 commits into
base: pharo-12
Choose a base branch
from
Open

Many gcs #653

wants to merge 20 commits into from

Conversation

PalumboN
Copy link
Collaborator

@PalumboN PalumboN commented Jul 18, 2023

Done in DojoVM - Douai edition (and more)!

Merged changes in #649 (I cannot point to it because it comes from a fork, even though we work in the same team 😞)

So, this PR includes:

  • Support for the new parameter in the CLI -> sets the GC at startup
  • Transform polymorphic calls into switch-case

HYBRID_COMPACTOR = 1,
SWEEP_COMPACTOR = 2,
SELECTIVE_COMPACTOR = 3
} GarbageCollector;
Copy link
Member

Choose a reason for hiding this comment

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

Ok so here we have some indices,

newWithExpression: (TVariableNode named: variableToTest)
selectors: (possibleClasses collect: [ :class |
class staticallyResolvePolymorphicSelector: aSendNode selector ])
arguments: aSendNode arguments
Copy link
Member

Choose a reason for hiding this comment

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

:)


self staticallyResolvedPolymorphicReceivers
at: polymorphicVariable
put: { typeVariable } , classes
Copy link
Member

Choose a reason for hiding this comment

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

Maybe in a following iteration, we should have something better than an array here?

@@ -31,7 +31,7 @@ Class {
'VMBasicConstants',
'VMSpurObjectRepresentationConstants'
],
#category : #'VMMaker-SpurMemoryManager'
#category : #'VMMaker-InterpreterSimulation'
Copy link
Member

Choose a reason for hiding this comment

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

Why moved to simulation?

[:selectorToStaticallyResolve|
aCCodeGenerator
staticallyResolveMethodNamed: selectorToStaticallyResolve
forClass: self
to: (self staticallyResolvePolymorphicSelector: selectorToStaticallyResolve)]]]
to: (self staticallyResolvePolymorphicSelector: selectorToStaticallyResolve)]]
Copy link
Member

Choose a reason for hiding this comment

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

Is this still needed?

Comment on lines +6804 to +6810

" gcType caseOf: {
[ 0 ] -> [ planningCompactor postSwizzleAction ].
[ 1 ] -> [ sweepCompactor postSwizzleAction ].
ONLY SELECTIVE
}.
"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
" gcType caseOf: {
[ 0 ] -> [ planningCompactor postSwizzleAction ].
[ 1 ] -> [ sweepCompactor postSwizzleAction ].
ONLY SELECTIVE
}.
"
compactor postSwizzleAction.

?

from: ObjStackFreex
to: ObjStackNextx + (self rawHashBitsOf: stackOrNil).
to: ObjStackNextx + (self rawHashBitsOf: stackOrNil) ].
}.
Copy link
Member

Choose a reason for hiding this comment

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

Ok, this one needs polymorphism :)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, actually not, if we are here we know the compactor is always a planning compactor. Maybe we need to move this method to the compactor

from: ObjStackFreex
to: ObjStackFreex.
to: ObjStackFreex ].
}.
Copy link
Member

Choose a reason for hiding this comment

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

same

@@ -69,6 +70,7 @@ EXPORT(int) vm_init(VMParameters* parameters)
setMaxOldSpaceSize(parameters->maxOldSpaceSize);
setDesiredEdenBytes(parameters->edenSize);
setMinimalPermSpaceSize(parameters->minPermSpaceSize);
setGarbageCollectorType(parameters->garbageCollector);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
setGarbageCollectorType(parameters->garbageCollector);
setGarbageCollectorType(parameters->garbageCollector);

parameters->garbageCollector = PLANNING_COMPACTOR;

return VM_SUCCESS;
}
Copy link
Member

Choose a reason for hiding this comment

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

All the changes in this file are hidden in the re-formatting. Even hiding white spaces in the PR shows a lot of noise... It would have been nice to not have it...

@PalumboN PalumboN added the dojo Things developed on VM Dojos label Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dojo Things developed on VM Dojos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants