On 22/03/12 12:00, Matthew Toseland wrote: > On Wednesday 21 Mar 2012 00:30:42 Ximin Luo wrote: >> "Implicit dependency" means that changing some code in one area results in a >> logical error in the other, that is not immediately apparent in the >> documentation nor any compile-time checking built into the language. >> Obviously >> this is not necessarily a bad thing, but there is a lot of it in the config >> system, and elsewhere, including in how the configured values are used. > > Okay... >> >> For example, for the longest time there was no centralised management of >> run-time files used by Freenet, and even now they are just a bunch of extra >> fields in Node.java because I had no existing structure to fit such a system >> into. However, Node.java is such a fucking disorganised mess, with several >> hundred fields, that it's not immediately obvious to everyone that they >> should >> be using "new File(Node.nodeDir(), fileName)" rather than "new >> File(fileName)". >> I cleaned it up when I first wrote that part, but it looks like misuses have >> slowly crept in again. > > So your complaint is that there is no single central point to put all the > folder names in? Feel free to create one. > > Making stuff static is tempting but means we can't do multi-nodes-in-one-VM > tests, although I believe classloader hacks can solve that? Is there much of > a performance cost? Otherwise we can just use a global object and grab it > from Node, or ClientContext. >> >> Yes, configs have EVERYTHING to do with "writing the values out". All the >> read/write logic is wrapped up into a ConfigCallback (which throws a >> InvalidConfigException) which is ONLY PASSED TO THE CONFIG SUBSYSTEM. Even >> ignoring the obvious intention of whoever named these classes, the target of >> the read/write logic is completely detached from the logic itself. > > ConfigCallback doesn't do any writing out. And the getters don't throw. It > can only throw when a value is changed. So it's still not clear what the > problem is.
Irrelevant. To repeat: - the target of the read/write logic[1] is completely detached from the logic itself[2], and too strongly-coupled with the config system[3] - too much implicit dependency, due to the way the variables are poorly managed [1] whatever get/set acts on [2] ConfigCallback [3] via .register(), so only the config system sees it Who said anything about making stuff static? Stop making up random straw mans. "Feel free to create one" - stop making excuses for not cleaning up your shit and telling someone else to do it when they complain! Node.java is a complete fucking mess. To make any sort of progress in having a well-structured config system, that needs to be cleaned out first. >> >> Your explanation about the ObjectContainer also assumes a severely stunted >> events framework can't support fine grained thread control. Why can't there, >> in >> your imagination, exist SOMEWHERE, a framework that lets you mandate "only >> run >> jobs of type J in thread T"? > > And those jobs are called with parameters (ObjectContainer, ClientContext) ? > Since we'll have to subclass it anyway, it's going to be more messy than the > current solution, isn't it? Anyway DBJob/DBJobRunner is pretty simple > (although it's hidden in NodeClientCore along with ClientContext > initialisation which is kind of messy). > This is bullshit, there is already a "DBJobWrapper implements Runnable" which needs nothing extra passed in. In the general case, there are *always* ways to get around fiddly crap like this, and they are much simpler than WRITING YOUR OWN EVENTS FRAMEWORK (and logging framework and config framework etc etc etc). > I agree we should make *Executor use the standard APIs though - while keeping > the ability to change thread priorities. >> >> An events framework could handle errant plugins by setting a timeout for all >> blocking jobs. If they don't complete within X time, interrupt the thread >> and/or kill it if it doesn't respond to the interrupt. If the plugin can't >> handle it, tough shit their fault. (This is how certain companies run >> reliable >> services on a massive scale.) > > Can't be done. Period. > > It's Java. Java does not support killing threads. According to the > documentation the reason for this is that killing a thread might cause > synchronization problems. Which is true, but I would have thought it was > solvable ... Anyway if you need to be able to kill stuff it has to run in *a > separate JVM*. Then interrupt is fine and covers most cases. >> >> As for the updater, simply seeing where the currently-running JARs are, is >> not >> enough, if an update introduces extra JARs as a dependancy. It's necessary to >> have an explicit list of all the dependencies. Unfortunately it's not enough >> to >> simply use the Class-Path attribute of the manifest of freenet.jar, because >> that hard-codes the path to the other JARs, whereas we need it to be variable >> for packaging purposes. So, this information can only be figured out at >> *package time* (for arbitrary package layouts), and the updater needs a >> mechanism to read this information, and probably update that as well. > > You are planning to have a packaged version which is able to > auto-update-over-Freenet? > > Currently class-path hardcoded freenet-ext.jar for convenience more than much > else. 99% of installs use the wrapper. Any software package should support being installed into any arbitrary layout. It's just good practise. >> >> X >> >> On 20/03/12 23:18, Matthew Toseland wrote: >>> On Monday 19 Mar 2012 01:52:19 Ximin Luo wrote: >>>> On 16/03/12 18:13, Matthew Toseland wrote: >>>>> On Thursday 15 Mar 2012 21:02:26 Ximin Luo wrote: >>>>>> (Top-posting because previous post is too long to reply to in-line.) >>>>>> >>>>>> ## refactoring >>>>>> >>>>>> Refactoring the code helps to make it more understandable for other >>>>>> coders. One >>>>>> of the reasons why I don't work more on freenet myself is because it's >>>>>> extremely difficult to make any changes that are more than a simple bug >>>>>> fix. >>>>> >>>>> Hmmm, good point. But from FPI point of view, unless it's a very quick >>>>> and high impact refactoring, it's probably too long term. >>>>>> >>>>>> When I was writing code for the config subsystem to separate out >>>>>> different >>>>>> files into different directories, this was very frustrating and took up >>>>>> much >>>>>> more time than I expected. >>>>> >>>>> I'm still curious as to why. >>>> >>>> The code is spread out over many files not related to config management; >>> >>> It is? How so? >>> >>> The code for handling the config files simply takes a value and applies it >>> to that subsystem. Short of converting it to get*/set*'ers, using >>> reflection, I don't see how we could make it much simpler. >>> >>>> this >>>> creates lots of implicit dependencies, which is not directly apparent in >>>> the >>>> syntax/structure of the code nor the semantics of the languag. In other >>>> words, >>>> spaghetti code. >>> >>> I still don't follow - implicit dependancies? >>>> >>>> Config management code should not be mixed into the thing they are >>>> configuring, >>>> this is just Good Programming. >>> >>> I don't get it. Evidently I didn't take that course. >>> >>>> Changing variables on-the-fly is a completely >>>> separate issue from config management and actually writing out those >>>> values, >>>> and it was a mistake to bundle the two together in the code. >>> >>> The code that is "spread out over many files" has nothing to do with >>> writing the values out. All it does is changes the values. Sometimes it is >>> necessary to store a copy of the configured value so that it can be >>> returned exactly, most of the time it just gets and sets a variable. >>>> >>>> Using the guice framework would help this a lot as well. >>> >>> I am not convinced. >>>> >>>>>> >>>>>> NB: I'm not implying you should be doing all the work toad, please don't >>>>>> think >>>>>> this. And please don't think that my opinions are automatically invalid >>>>>> / less >>>>>> worthy just because I haven't committed code for ages. I just want to >>>>>> express >>>>>> my view of What I Would Do If I Were God. >>>>> >>>>> Okay. :) I don't mean to criticise un-constructively, just have a useful >>>>> conversation. >>>>>> >>>>>> People can do what they want but it doesn't mean it's a good idea. >>>>>> Granted, I >>>>>> consider "code quality" independently of any issues such as funding, but >>>>>> focusing too much on the latter leads to bad software. If there's >>>>>> pressure like >>>>>> this, ignore it, find something else in the meantime until it goes away. >>>>> >>>>> I plan to continue working for FPI for the time being. Partly because >>>>> it's part time and fits conveniently with studying. Partly because >>>>> getting a programming job in the UK requires a degree even if you have >>>>> some rather odd experience. >>>>>> >>>>>> ## freenet Events API >>>>>> >>>>>> freenet does not provide a great API for running callbacks and scheduled >>>>>> tasks. >>>>> >>>>> I'm not sure what you mean here. For example, the client layer has unique >>>>> requirements. We simply can't just change it to use java.util.concurrent. >>>>> Database jobs must run on the database thread, and no other thread may >>>>> have access to the database, because the client layer is not designed to >>>>> deal with simultaneous transactions. This avoids performance problems >>>>> (separate caches for each client; simultaneous disk I/O), and complexity >>>>> (refreshing everything constantly, which requires a more query-oriented >>>>> architecture; we'd have to change pretty much everything, and it'd >>>>> probably be slower). But it's messy, in particular it requires passing >>>>> around ObjectContainer's. >>>> >>>> Why can't these restrictions be handled by a 3rd-party events framework? >>> >>> They can't. Period. >>> >>> The long answer is you can't pass an ObjectContainer to any other thread, >>> and any DBJob needs both the ObjectContainer and the ClientContext (which >>> isn't stored in any persistent class and is effectively the means to get >>> all the objects created for this run of the node that aren't persisted >>> otherwise, plus some global stuff like the FEC queue). >>> >>> The simplest way to enforce this rule is simply the ugly convention of >>> always passing the ObjectContainer in, and NOT either making it a singleton >>> global or storing it anywhere. >>> >>> The expensive way to deal with this would involve even more unwritten >>> rules: You could write a wrapper for ObjectContainer that throws if you try >>> to access it from another thread, for example. >>> >>> The really expensive way to deal with this is to have parallel >>> transactions. However, this way lies madness, because: >>> 1. Typical end-user PCs' disks may be slower rather than faster with more >>> parallel transactions. >>> 2. More importantly, there is *one cache per transaction*. >>> 3. Code-wise, we would have to refresh everything every time we use it. >>> Although we already have a lot of activation and deactivation, so maybe >>> that's less of an issue. We would need to reconsider the "roots", and >>> probably make everything start with a query. Db4o queries generally are >>> rather slow so this would probably be either slow or messy (e.g. creating >>> key objects using strings etc, like p0s does in Freetalk and WoT). >>> 4. We would also have to deal with rollbacks. Arguably these are a good >>> thing - p0s uses them - but that would mean that some easy optimisations >>> because extremely difficult. Once these issues are solved for Freetalk/WoT >>> (e.g. by means of a block level cache capable of caching more than one >>> transaction and then writing them to an alternate file, and alternating >>> between the two), it might be worth considering, but I doubt it would be >>> worth it. >>>> >>>>>> Converting everything to use java.util.concurrent and/or >>>>>> com.google.common.util.concurrent would help this a lot. Of course, >>>>>> Library can >>>>>> and currently does implement this itself, but it's fairly sloppy, and >>>>>> other >>>>>> plugins would benefit if such a framework were provided. >>>>> >>>>> If you simply mean replacing Ticker and Executor, that's fine by me. >>>>>> >>>>>> Some common tasks that plugins would like to do, which really should be >>>>>> provided by the underlying application: >>>>>> - run tasks in the background >>>>>> - run tasks according to a particular schedule >>>>>> - cancel running tasks >>>>> >>>>> How do you propose to cancel a task once it's started? I guess it depends >>>>> what sort of task it is. If it has a boolean and periodically checks it >>>>> then fine; this would require a subclass ... >>>> >>>> Depends on what the task is. The writer of the "task" will need to add >>>> code to >>>> support cancellation, just like Future.cancel(). >>> >>> Okay. >>>> >>>>>> - handle dependencies between tasks, so that e.g. if A depends on B and I >>>>>> cancel B, A is automatically cancelled. >>>>> >>>>> That's a nice bit of functionality yeah. Nothing in fred needs it at >>>>> present, although radical refactorings might make it more useful. >>>> >>>> Having this functionality would make Library's code a lot simpler. (The >>>> dependency management that is, not necessarily the cancellation.) We talked >>>> about it a while back but I never really sat down to solve the issue. >>> >>> I have no problem with importing a small third party library or even having >>> such classes in freenet/support/. >>>> >>>>>> - group tasks into related categories so one group doesn't affect the >>>>>> other. >>>>>> (e.g. tasks for the group "Library/b-tree-write/index-<URL>" won't >>>>>> starve the >>>>>> resources of group "WoT/background-trust-calculation") >>>>> >>>>> If you are scheduling tasks in a limited pool this makes sense. Freenet >>>>> generally doesn't do this because most tasks are either CPU intensive and >>>>> complete quickly, or are blocking / I/O intensive and take ages but don't >>>>> use much resources. Also many tasks are latency sensitive. And on modern >>>>> JVMs, lots of threads are cheap, although memory is an issue, we do need >>>>> to keep it down if possible ... >>>> >>>> There is a thread limit. You don't want plugin A to hog 90% threads and >>>> leave >>>> the other 10% to everything else. >>> >>> It's different for plugins IMHO. Although there is only so much you can do >>> to guard against errant plugins. >>>> >>>>>> >>>>>> ## config management >>>>>> >>>>>> Code for read/writing the config in embedded inside the classes they >>>>>> control, >>>>>> which clutters up the code and makes it confusing. Config code should be >>>>>> separated from the actual application. It would also be nice if this was >>>>>> exposed to plugins. >>>>> >>>>> Separated how exactly? You want to call getters via reflection, like java >>>>> beans? It is always going to be the case that changing some settings on >>>>> the fly will be somewhat involved and require methods for it; the classes >>>>> are only one way to write such an interface, I'm open to other mechanisms >>>>> given that config happens very infrequently. >>>>>> >>>>>> Being a daemon is why the current config system is NOT SUFFICIENT. I >>>>>> need to be >>>>>> able to lock certain config settings, such as where to keep the >>>>>> datastore / >>>>>> run-time files themselves, to conform to the FHS. It's best that such >>>>>> settings >>>>>> are kept in a separate read-only file. The current system only reads one >>>>>> file, >>>>>> freenet.ini. >>>>> >>>>> Sounds to me like that could be achieved with some fairly small changes; >>>>> I still don't see what the problem is. >>>> >>>> Adding an "#include" mechanism for config files whilst supporting "write" >>>> at >>>> the same time is not a "small change". Think about it. >>> >>> There are several cheap and dirty options, for example, we could provide a >>> list of config files on the command line, with changes being written to the >>> first (or the last, possibly configurable with another command line option). >>> >>> But yes, maybe we need something more elegant. >>> >>> But *we need to be able to write to a config file*. Ease of use on the >>> majority platform (Windows) makes this an absolute requirement. We cannot >>> get rid of it, period. >>>> >>>> (This is why .gitconfig doesn't have #include whereas .hgrc does.) >>>> >>>>>> Updating its own binaries is not a problem if freenet knows where the >>>>>> binaries >>>>>> are. The current installer puts them in a rigid place, however this is >>>>>> incompatible with FHS. >>>>> >>>>> A rigid place? I don't follow. If you are doing FHS you are using a >>>>> package; the installer is never going to comply with FHS as, apart from >>>>> anything else, it shouldn't be run as root! >>>> >>>> The updater (UpdateDeployContext) makes some very specific assumptions as >>>> to >>>> the locations of the jars, and this method doesn't generalise to non-jar >>>> files. >>>> It also assumes wrapper.conf is in the current directory. >>> >>> I believe we can ask the wrapper where the file is - and probably the jars >>> too. >>> >>> However I thought you were expecting to just turn off auto-update in >>> packaged versions? >>>> >>>>> Updating its own binaries is incompatible with the standard unix way of >>>>> doing things, isn't it? Even if it's not technically a violation of FHS? >>>>> >>>>> >>>>> _______________________________________________ >>>>> Devl mailing list >>>>> Devl at freenetproject.org >>>>> https://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl -- GPG: 4096R/5FBBDBCE https://github.com/infinity0 https://bitbucket.org/infinity0 https://launchpad.net/~infinity0 -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 900 bytes Desc: OpenPGP digital signature URL: <https://emu.freenetproject.org/pipermail/devl/attachments/20120322/0110bf97/attachment.pgp>
