Sorry about the late reply to this. I wrote up the first 2/3 immediately after seeing Josef's message, but got sidetracked by Real Life. Then when I came back to it, I noticed that it had already exceeded the length threshold that usually means I'm rambling incoherently. And I probably am, so you can blame Piers Cawley for mentioning this in the summary and guilting me into finally sending this.
On Mon, Sep 09, 2002 at 05:12:01PM +0200, Josef Hook wrote: > > I've seen that intlist adds a file called intlist.c to the core of > parrot. This IS! strange to me that Steves patch made it into > parrot while my Matrix patch didnt just because i had a file in the core > of parrot called matrix_core.c. Dan wanted everything in one file. > Why should my matrix code live in only one file while both perlhash.pmc > and intlist and coroutine and others use external code. > I feel that its VERY hard to contribute as an outsider. If parrot > wants to grow to a serious VM it has to accept other peoples patches. Well, the real answer in this case is that it made it in because I have commit privileges and I unilaterally decided to put it in. Well, I let people comment for a while first. But that answer doesn't make your question invalid, it just splits it into two parts: (1) should I be able to do that, and (2) should I have used that power in this particular case? For (1), any answer that pertains only to me is of course going to be highly biased, so I will address it more generally: should there be people who have the right to commit whatever they want, and are the criteria we use for selecting those people justified? Clearly, many projects do NOT have such people -- take the Linux kernel, for example. Or even perl5. I don't know how many people have commit rights for the perl5 tree, but it seems to me like it's pretty much all gated through the pumpking. In Parrot, anybody who has a history of quality patches can ask for and gain commit rights. Both models seem to work, although I would argue that Parrot would never get anywhere if it used a single-committer model because we don't really have a single person who has the time and bandwidth necessary to accurately review and apply all the patches. And Parrot is still a young bird, so it's not like there is a huge established base that will suffer from the sporadic upheavals. It's better to get more of the basic functionality in than to make absolutely sure that things are implemented in just the right way. Later on, a different model might make more sense for a more mature system. But I also need to argue the other way -- why should there be people who have more rights than others? For that, I can only point to the dangers of allowing free access to anyone who wanders by. We programmers are notorious for overestimating our own abilities, and opening up the tree for commits by anyone would quickly result in a tangled mess of good intentions gone horribly bad. And also notice that those of us with commit privileges rarely use them for our own code, except when it's in an area that we are the de facto "maintainers" of. There are exceptions, and I probably figure larger in those exceptions than anyone else, but I try to be very sensitive to what might be controversial and what's just a clear win. As for (2), I can only say that I believe my patch to be of general enough applicability that it deserves to add a core file. It implements a capability likely to be useful for things other than PMCs (and, in fact, is intended to cause the *removal* of rxstacks.c; I just didn't want to do both at once.) And the PMC itself implements a stated language feature, in this case a packed integer array. Also, integer stacks appear necessary for a key required capability of Parrot: regular expressions that are faster than perl5's. (That's why rxstacks.c made it in, too, and intlist.c is really just an improved implementation.) In regards to why your multiarray patch took so long to make it in, I can't speak for Dan. But I can speculate! Multiarrays aren't a necessary core feature of a VM, and there are many different ways of implementing them, so it would be best if a trial implementation were as self-contained as possible. The core code should be fairly narrowly focused on what is necessary to implement Parrot; the only place we've come up with so far for experimentations with non-core features is the PMCs. We'll need more (e.g., dynamic PMC and op loading), but that's all we have for now. I personally think we ought to at least let more complex PMCs have a directory of their own to play in, but someone has to propose and implement that first. (And there may be very good reasons for shooting that idea down, I don't know.) For all I know, maybe Dan didn't want you committing the matrix_core.c file because your code was clumsy and amateurish, but he didn't want to discourage you either. I don't know; I haven't read the code because I haven't needed it for anything yet. I have responded at such length both because as someone with commit rights, I feel like I am responsible for justifying my use of them, and because of these sentences: > I feel that its VERY hard to contribute as an outsider. If parrot > wants to grow to a serious VM it has to accept other peoples > patches. I can't let that pass unchallenged. I know you're annoyed at how much work was required to get your stuff in, but I disagree with your conclusion. You need to be aware that your chosen route to contribution was an ambitious one: you started out with a very large patch, without having first eased your way in by improving on existing functionality. That is not by itself bad, but it does make it harder to be sure that you'll stick around and maintain it, and not just drop it in and wander off after your interest wanes. (Although by now I think you've proven your staying power!) Even that wouldn't be a big deal if it were just an isolated PMC that could be dropped painlessly if its maintainer disappeared and nobody was using it. Actually, the very notion of "maintainer" for a particular chunk of code is dangerous and better avoided in an open-source project like Parrot -- if only one person works on that chunk of code, it may mean the code is either too convoluted to be understood, or too rarely used for people to care. Either way, it's a breeding ground for bugs, and runs the risk of confusing newcomers who look to it for examples. (Note that this part does not apply to the multiarray stuff yet, since *somebody* has to be the original author, and that somebody will have to maintain the conceptual integrity for a while.) That was just a hint to anyone who might be reading that going over existing code is often an even better way of contributing to Parrot than implementing some new feature. Finally, I should mention that in my personal opinion, if nobody starts using intlist for interpreter internals, then I think it probably should be moved into the classes/ subdirectory or a subdirectory thereof. (Oh, and as long as I'm throwing around dumb ideas, I think classes/ should be renamed pmcs/ too.) To maintain the maximum possible speed for regexes but again without doing nasty regex-specific hacks, I'd like to see something like the assembly syntax set I0, P0:IntArray[7] supported. Specifically, I want a way to tell the assembler the actual dynamic type of a PMC register, so it can avoid an indirection through the vtable. Even better, the JIT can make use of this to implement its own PMC functions. It would be nice if IMCC could propagate this type information when possible, but we'd have to put some kind of restrictions on what certain operations could do (like "new P0, ..IntArray will always return an IntArray, not some subclass or unrelated class". And operations could be declared to never change the type of their PMC arguments.) They can feed off each other, too -- perhaps 'set P0, I0' cannot be guaranteed to leave the type of P0 alone, but 'set P0:IntArray, I0' could, so if you know that P0 won't change type up to that instruction, then you know it will maintain its type through the instruction. Enough babbling for now.
