VMMaker small problem for pharo developper

hi,
I have see the immutability bit implementation and i think, it can be easily integrate in the actual VM, the main problem is VMMaker, especially the difference existing between Pharo and Squeak, using VMMaker on Pharo imply to patch it (only small difference, deprecated method, changing method name ), then the main problem is i can change the VM on pharo without creating a incompatible VM for squeak(cause of the patching).

Then i just want to find a solution to have exactly the same version of VMMaker on squeak and pharo. then if someone want to contribute, he can, regardless of this environment .

Then i want to contribute, but i can't.

any solution ?

Regard
Jean Baptiste Arnaud
[hidden email]

On 18 March 2010 12:11, Jean Baptiste Arnaud <[hidden email]> wrote:
VMMaker is a standalone project, maintained by vm-dev group and using
http://www.squeaksource.com/VMMaker
repository for development, which then used to build VMs.

So, all you have to do is to use it for development and contribute to it.

> Regard
> Jean Baptiste Arnaud
> [hidden email]
>


--
Best regards,
Igor Stasenko AKA sig.

On Mar 18, 2010, at 11:20 39AM, Igor Stasenko wrote:

Almost, but not entirely.
Certain plugins have their methods implemented in the base image, such as MiscPrimitivePlugin>>primitiveFindSubstring .
When the method containing the actual implementation is renamed (as was done in Pharo, and is what Jean Baptiste refers to) the translatedPrimitives mapping in VMMaker no longer works.

The renaming was done to avoid changing all users of findSubstring: in: startingAt: matchTable: , since the primitive returns erroneous results if the substring is a WideString.
In Squeak, (well, code is the same in Pharo actually), senders have instead been rewritten to contain guard clauses checking classes of arguments...
Let's just hope someone not familiar with the bug decides to use the method for something else, and forgets to add a clause :)

Cheers,
Henry

Then what is the solution ? We could have a Pharo immutability bit VM, but that it can be better to share with squeak. (which should be easy, if these differences are fixed ).

On Mar 18, 2010, at 1:06 PM, Henrik Johansen wrote:

Regard
Jean Baptiste Arnaud
[hidden email]

On Mar 18, 2010, at 1:26 10PM, Jean Baptiste Arnaud wrote:

>
> Then what is the solution ? We could have a Pharo immutability bit VM, but that it can be better to share with squeak. (which should be easy, if these differences are fixed ).

I agree.

One solution for the case I mentioned is to not hardcode where the primitives in such cases as MiscPrimitivePlugin are defined, but instead discover them in the image VMMaker is loaded into.
Thus you remove the link between VMMaker, and the image it's loaded into defining primitives in certain places, so the same VMMaker can work for both Squeak and Pharo as described.

F.ex. like this:

MiscPrimitivePlugin class>>translatedPrimitives
oc := OrderedCollection new.
CompiledMethod allInstancesDo: [:each |
        (each pragmaAt: #primitive:module:) ifNotNil: [:prag |
                ((prag  argumentAt: 2) = 'MiscPrimitivePlugin'  and: [each literals includes: #var:declareC:])
                        ifTrue: [ oc add: (Array with: each methodClass name with: each selector)]]].
^oc asArray

This assumes the method defining the primitive will include the primitive pragma, which should always be the case.
The "each literals includes: #var:declareC:" is a bit of a hack for disambiguating the CompiledMethod defining the primitive from other callers.
AFAIK, hints like that are required if the method is to be translated, and you are unlikely to find it in other callers. If anyone has a better idea, I'd appreciate it.

Cheers,
Henry

On 18 March 2010 14:06, Henrik Johansen <[hidden email]> wrote:
Sorry, i missed the whole discussion around that fix, so i can't guess
what would be best solution.
Or why you had to make VM to behave differently than for Pharo than for Squeak.
My only guess, that a proper solution would be to propose fix for VM,
then do appropriate fixes in squeak and pharo.

Otherwise, we're going to fork VMs, with all consequences.

> Cheers,
> Henry
>
>



--
Best regards,
Igor Stasenko AKA sig.

 
On Thu, Mar 18, 2010 at 02:08:36PM +0100, Henrik Johansen wrote:
Henry,

Thank you for opening a mantis report on this at
  http://bugs.squeak.org/view.php?id=7479

I think your pragma idea may work, but IMO this is getting too
complicated.  Perhaps the change in Pharo was done without realizing
the problem it would cause for C code generation. So that means
that the primitive is already too clever - it is easy for someone to
look at the Smalltalk method and not realize that it is designed to
be translated to C. I am afraid that adding pragmas will only make
it harder for people to understand how this works.

So my opinion FWIW is that if we really need different methods in
the Squeak and Pharo images, then maybe it is time to reimplement
this primitive as a normal primitive, rather than using the magic
of MiscPrimitivePlugin translating methods directly from the image.

But to be honest I am not convinced that the Pharo method really needs
to be renamed to #findSubstringViaPrimitive:in:startingAt:matchTable:.
Perhaps there is some other way to accomplish this refactoring without
causing compatibility problems?

Dave

On Mar 19, 2010, at 4:48 27AM, David T. Lewis wrote:
Well sure, I doubt that was a consideration.
However, imo it is the *right* way to fix the issue, by keeping the contract of the public method, but factoring out the act of calling the primitive (which may not fail, but return erroneous results), and implementing the guard in the "old" primitive method.
What do you think will happen in Squeak if an external package uses findSubstring: directly? I sincerely doubt they will be aware of its limitations, and have the proper guards in place.

> So that means
> that the primitive is already too clever - it is easy for someone to
> look at the Smalltalk method and not realize that it is designed to
> be translated to C. I am afraid that adding pragmas will only make
> it harder for people to understand how this works.

I suppose you refer to adding pragma usage when resolving locations, since the pragmas themselves are already there.

Using them for resolving was merely the simplest way I could think of which would resolve the primitive definition location for VMMaker, while not hardcoding them.

In sentences describing the functionality of translatedPrimitives, it changes from "These are the hardcoded locations where my methods are defined" to "My methods are defined in places where my primitives are called, and there also exists c-declaration hints".
Personally, I don't really see how that is much harder to understand.


>
> So my opinion FWIW is that if we really need different methods in
> the Squeak and Pharo images, then maybe it is time to reimplement
> this primitive as a normal primitive, rather than using the magic
> of MiscPrimitivePlugin translating methods directly from the image.

Sure.
That'd probably also be a good change in the sense you don't need to know which image VMMaker was loaded into when the plugin was built to know how it works.
How do you redirect images where the MiscPrimitivePlugin method is called, and not the primitive though?
>
> But to be honest I am not convinced that the Pharo method really needs
> to be renamed to #findSubstringViaPrimitive:in:startingAt:matchTable:.
> Perhaps there is some other way to accomplish this refactoring without
> causing compatibility problems?

If you mess with findSubstring:in:startingAt:matchTable: to take care the WideString case is handled correctly, AFAICT you inevitably end up with a solution which would either mean:
- It does not end up using the primitive it defines.
- It breaks building the primitive with VMMaker.

So the only other option, is to put  guard clauses in all senders...
Squeak does it. As I noted, in fact  Pharo does it as well, but shouldn't, as it already have them centralized in the old findSubstring: method.
To me, it does not seem like the right way though, unless your image is static ,and you can ascertain noone else will ever call findSubstring:in:startingAt:matchTable: .

Cheers,
Henry

PS.
FWIW, as discussed in an earlier thread, the method will probably be renamed again in Pharo, to #primFindSubstring:in:startingAt:matchTable:

On 19.03.2010, at 10:44, Henrik Johansen wrote:
>
>
> the method will probably be renamed again in Pharo, to #primFindSubstring:in:startingAt:matchTable:

I see no compelling reason why we shouldn't just apply the same refactoring to Squeak? KISS ;)

- Bert -

 

On 19.03.2010, at 11:08, Henrik Johansen wrote:
It sounds sensible to me, and there is no policy in place to Just Be Different ;)

I bet if you published a package with the refactoring to the trunk inbox (open to submissions for all) it would very soon be merged.

> VMMaker would have to be changed either way to stay compatible :)
>
> Cheers,
> Henry


- Bert -

 
On Fri, 19 Mar 2010, Bert Freudenberg wrote:

>
> On 19.03.2010, at 10:44, Henrik Johansen wrote:
>>
>>
>> the method will probably be renamed again in Pharo, to #primFindSubstring:in:startingAt:matchTable:
>
> I see no compelling reason why we shouldn't just apply the same refactoring to Squeak? KISS ;)

What about changing the primitive to fail if the String is "Wide", or make
it work with WideStrings?


Levente

>
> - Bert -
>
>

On Mar 19, 2010, at 1:18 17PM, Levente Uzonyi wrote:

Sure, it's an orthagonal issue though.
You'd still need the guards in case an old VM is used, and have to skip those if the image the VM was built with included the primitive code which works correctly.
IE. it'd be a real mess, and you'd definately want to have to do it in only one place.

Cheers,
Henry

 
On Fri, 19 Mar 2010, Henrik Johansen wrote:

I'd call it parallel, not orthogonal. If the primitive is fixed, then in
Squeak 4.2/Pharo 1.2/etc we can remove the checks/hacks in the image and
assume that the issue is handled correctly by the vm.


Levente

>
> Cheers,
> Henry
>
>

 
2010/3/19 Levente Uzonyi <[hidden email]>:
... or the primitive fallback code.
I agree with Levente, the first thing to do is to protect the primitive

Nicolas

>
> Levente
>
>>
>> Cheers,
>> Henry
>>
>>
>