Mantis Bugtracker
Main | My View | View Issues | Change Log | Docs
  

Viewing Issue Simple Details [ Jump to Notes ] [ View Advanced ] [ Issue History ] [ Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0006241 [Squeak] Monticello major always 02-27-07 17:54 10-03-09 19:34
Reporter RalphJohnson View Status public  
Assigned To andreas
Priority normal Resolution open  
Status assigned   Product Version 3.9
Summary 0006241: Atomic loading
Description MC loads methods one at a time, loading one before it compiles the next. Packages that change the compiler (or MC) or classes that they use often do not load. Programmers can edit change sets to make sure that methods get changed in the right order, but there is no way to control the order of loading methods in a MC package. This is one of the disadvantages of MC relative to change sets.

This attachment implements atomic loading. It adds a class called MethodAddition that represents a method in its various stages of being added. First, it gets compiled. Once all methods have been compiled, the PackageLoader will install all of them in their classes. There is one method in PackageLoader that does this; it uses OrderedCollection>>do: and a few methods from CompiledMethod, but otherwise doesn't depend on anything. Finally, all the methods signal that they have been updated, so that tools can update their views.

I have not yet implemented atomic unload. I'll do it when I need it. Let me know if you need it.

-Ralph Johnson johnson@cs.uiuc.edu
Additional Information
Attached Files  Monticello-rej.309.mcz [^] (199,039 bytes) 02-27-07 17:54
 Foo-lr.1.mcz [^] (957 bytes) 03-01-07 08:17
 Foo-lr.2.mcz [^] (1,029 bytes) 03-01-07 08:18
 MCLoadMapTest.st [^] (1,033 bytes) 03-09-07 08:05

- Relationships
related to 0007017closed Keith_Hodges Sometimes Classes are not recompiled when instance variables change 

- Notes
(0010312 - 153 - 153 - 153 - 153 - 153 - 153)
renggli
03-01-07 08:16

 I tried you enhancement and see no big improvement, after all I would certainly not call it atomic. To reproduce load Foo-lr.1.mcz and then Foo-lr.2.mcz.
 
(0010440 - 362 - 410 - 410 - 410 - 410 - 410)
pmm
03-09-07 08:09

 This Monticello can not load mcm, see the attached MCLoadMapTest.st.

Additionally it harvests none of the Impara fixes like:
- ignore the order of class variables
- loading FFI external structures
- ui fixes
-- bring the tree for dependent packages back
-- provide more progres information
-- don't scale the toolbar then resizing the monticello browser
 
(0010692 - 1053 - 1089 - 1089 - 1089 - 1089 - 1089)
RalphJohnson
05-10-07 18:41

 The problem that ranggli reports is different from the one I tried to solve.

His test case has a local variable named foo, and then he tries to define an instance variable named foo and then change the local variable name to be something else. However, it results in an inconsistent intermediate state. First, you have to rename the local variable and then you have to redefine the class. A changefile can handle this because it can ensure that the changes are done in that order. MC will usually not do it right, and my version of MC is certain to not do it right because it always compiles class definitions before method definitions. To do this, you need two different changes. This proves that you cannot always merge MC files to produce a working MC file.

A change is a program transformation, not a set of methods. Composing transformations is not the same thing as taking the union of the method changes.

So, my conclusion is that my enhancement is an improvement, and that renggli's test case is not appropriate for Monticello.
 
(0010693 - 305 - 329 - 329 - 329 - 329 - 329)
pmm
05-10-07 19:08
edited on: 05-10-07 19:10

 As pointed out several times, Monticello 2 has atomic loading and handles the test of Lukas just fine. All that had to be done is porting the class builder from Monticello 2 to Monticello 1.

There are also some nice papers of the VW guys (Elliot and the gang) about atomic loading in VW with parcels.
 
(0010694 - 878 - 902 - 902 - 902 - 902 - 902)
renggli
05-10-07 19:16

 The problem I report is a common problem when keeping up-to-date with ongoing development. I know of at least a handful cases where this unintentionally happened in the context of big industrial and open-source projects.

What I criticize on the proposed change is that the name is misleading. It is not atomic, because it still compiles incrementally. If the loader would be really atomic, then the order of changes wouldn't matter as everything would be there in one single uninterruptible shot: class addition/removal/recompilation, method addition/removal/recompilation, object migration, etc. The atomic loader of Colin Putney does exactly this and it doesn't fail with my test.

The signalling of the changes at the end and the way the change-logging is handled looks suspicious to break things. I am missing the tests showing that it actually does what you advertise.
 
(0010724 - 120 - 158 - 158 - 158 - 158 - 158)
Keith_Hodges
05-15-07 20:22

 "fix begin"
Installer mantis bug: 6241 fix: 'Foo-lr.1.mcz'.
Installer mantis bug: 6241 fix: 'Foo-lr.2.mcz'.
"fix end"
 
(0010843 - 427 - 451 - 561 - 561 - 561 - 561)
Keith_Hodges
06-29-07 07:51

 Update on status: I have added an MCPackageLoader2 to the version hosted at http://www.squeaksource.com/mc [^]

This loader uses the SystemEditor from MC2 and is therefore truely atomic!

However it appears that this use of SystemEditor has progressed further than the MC2 team have, and so SystemEditor has yet to be tested. So the down side being that the final step of committing the change to the system crashes the image.
 
(0010844 - 100 - 112 - 112 - 112 - 112 - 112)
renggli
06-29-07 07:55

 Keith, you are the man!

If you need any help for testing or bug-fixing, please send a mail to me.
 
(0012270 - 76 - 76 - 76 - 76 - 76 - 76)
Keith_Hodges
06-07-08 02:50

 I have tried the test case again with Monticello.impl-kph.515 and it passes!
 
(0012750 - 81 - 93 - 93 - 93 - 93 - 93)
Keith_Hodges
10-26-08 02:04
edited on: 10-26-08 02:04

 Atomic loading/unloading is working with MC1.6 (for non traits and tweak users)
 

- Issue History
Date Modified Username Field Change
02-27-07 17:54 RalphJohnson New Issue
02-27-07 17:54 RalphJohnson Status new => assigned
02-27-07 17:54 RalphJohnson Assigned To  => avi
02-27-07 17:54 RalphJohnson File Added: Monticello-rej.309.mcz
03-01-07 08:16 renggli Note Added: 0010312
03-01-07 08:17 renggli File Added: Foo-lr.1.mcz
03-01-07 08:18 renggli File Added: Foo-lr.2.mcz
03-01-07 08:18 renggli Issue Monitored: renggli
03-09-07 08:05 pmm File Added: MCLoadMapTest.st
03-09-07 08:09 pmm Note Added: 0010440
05-10-07 18:41 RalphJohnson Note Added: 0010692
05-10-07 19:08 pmm Note Added: 0010693
05-10-07 19:10 pmm Note Edited: 0010693
05-10-07 19:16 renggli Note Added: 0010694
05-15-07 20:22 Keith_Hodges Note Added: 0010724
06-29-07 07:51 Keith_Hodges Note Added: 0010843
06-29-07 07:55 renggli Note Added: 0010844
06-07-08 02:50 Keith_Hodges Note Added: 0012270
06-07-08 02:50 Keith_Hodges Status assigned => feedback
06-07-08 02:50 Keith_Hodges Status feedback => assigned
06-07-08 02:50 Keith_Hodges Assigned To avi => Keith_Hodges
06-07-08 02:52 Keith_Hodges Status assigned => feedback
06-07-08 02:57 matthewf Issue Monitored: matthewf
06-07-08 02:58 matthewf Relationship added related to 0007017
10-26-08 02:04 Keith_Hodges Note Added: 0012750
10-26-08 02:04 Keith_Hodges Note Edited: 0012750
01-10-09 01:53 Keith_Hodges Status feedback => pending
10-03-09 19:34 Keith_Hodges Status pending => assigned
10-03-09 19:34 Keith_Hodges Assigned To Keith_Hodges => andreas

Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
105 total queries executed.
54 unique queries executed.
Powered by Mantis Bugtracker