Ulf,
On the usage of List vs [], my point was that the clarity, simplicity
and safety
provided by iterating over a Collection out-weighs any performance cost
in this case,
as compared with using an array. It makes little difference to
the cost of creating an OS process.
On moving the SYSTEMROOT constant to be a local variable inside
toEnvironmentBlock(),
on balance it probably does look neater.
Since I have to regenerate the webrev to fix a problem I found with the
testcase, I'm including
that change and the change to the comment for emptyEnvironment(). But,
at this stage, I have
to move onto other work, and this is the last webrev for this fix,
unless someone finds a bug ..
http://cr.openjdk.java.net/~michaelm/7034570/webrev.5/
Thanks for the work you put into reviewing it.
- Michael.
Ulf Zibis wrote:
Am 18.04.2011 15:43, schrieb Michael McMahon:
Ulf Zibis wrote:
I think, the comment in lines 295..297 more belongs to the code
(lines 312..315 + 318..320 ) rather than to the constant SYSTEMROOT.
Why defining it as class member, as it is only locally referred?
It makes no difference to the generated code. And imo, there is more
space for the comment there.
Hm, can't share that reason. But anyway, the comment would be little
closer to the code, if the constant would at least be defined in the
mothod's scope.
I still don't see the advantage, having the SYSTEMROOT insertion
code at 2 call sites. One would be shorter and much clearer as shown
in my code. Then we too could save addToEnvIfSet(..) and the call to
it.
Shorter doesn't always mean clearer. I think this way is clearer.
Not always yes, but each code line has to be understood by the reader.
The more lines to read, the more lines to understand, apart from the
bigger memory and codebase footprint. Just my opinion.
Additionally: Why instantiating, initializing and processing a fat
j.u.List object, if a simple array would fulfil all needs?
I don't see a compelling reason to change it from a List (which is
what it was). I think any additional overhead
is inconsequential in the context of creating an OS process.
I don't understand. The overhead is bigger while using a List than for
a simple array.
Or if it is, then we should be looking at our List implementations.
If there would be thousands of processes to create, I would agree.
HotSpot would optimize to equivalent performance, but in real world, I
guess, VM's interpreter has to deal with the more overloaded code from
the List stuff.
-Ulf
