Patches to procletize System properties
Project: | JNode Core |
Component: | Code |
Category: | task |
Priority: | critical |
Assigned: | lsantha |
Status: | closed |
Jump to:
Description
The attached patches implement 'procletization' of the System properties, mark 'set' / 'propset' as 'internal' commands and change 'proclet' and 'isolate' invokers to respect the 'internal' flag. I've tested that the changes do in fact mean that different proclets (e.g. consoles) do have different System properties and that "cd" in one console no longer changes the current directory for other consoles.
Levente: please review these patches (per your request), make any changes you think are necessary, and apply them at your earliest convenience. "proppatch-1.txt" should be applied to the classlib tree, and "proppatch-2.txt" should be applied to the jnode tree.
Attachment | Size |
---|---|
proppatch-1.txt | 3.08 KB |
proppatch-2.txt | 9.91 KB |
- Login to post comments
#1
#2
While the patch doesn't look bad by a superficial inspection could you please give me two more days when I will take a closer look because currently I'm busy with my personal tasks not related to JNode.
#3
No ... that is not OK with me.
As I pointed out in another issue, your approach of us submitting patches to you for review is not sustainable. It gets in the way of making progress on JNode. And besides, as a developer with ~30 years experience, I find the whole process to be demeaning and degrading.
If you have a family emergency or something, the polite thing for you to do would be to commit the patch as-is (or give me the OK to commit it) and then make any changes you want to later on.
#4
I don't think I ask you, Steve, to submit a patch to me for review very often. This only happens in the most exceptional situations like this one.
I've reviewed the code to not hold you back.
The most important worry is that proclets are being pushed in the realm of isolates even more on the expese of overall system performance. For instance this patch makes System.getProperty() significantly more expensive (an extra 6-7 method calls or more) and this is a relatively often used method of the System class (see file path resolution and others).
You can commit it if you want and if you think it's very important but instead of pushing proclets everywhere isolates are the right answer and they would not required that performance overhead on the above method. The problem of proclets and isolates should be revisited later when isolates become more mature.
#5
Levente. Let me spell it out as simply as I can. It is offensive to imply that at an experienced developer is still not competent to work without your close supervision ... in my case after 1+ years working on JNode, and making over 1000 commits!
The performance impact of proclets on System.getProperties() has been discussed in another issue. It is likely to be a few hundred instructions if the proclet mechanism has been enabled, and maybe 10 to 20 machine instructions if it has not. If you want to take back your earlier request that proclets be enabled by default by the CommandShell ... it is trivial to do.
As it happens I cannot commit the patch immediately. I got home from work today to discover that my main dev machine won't boot due to memory errors. So I now have to set up Linux on this box, download Eclipse, JDK, JNode, blah blah. Grrrr!!!
#6
I have applies the patches and committed them along with the new classlib JARs.
Marking issue as fixed.
#7
Manually closed.