Metadata
- Source
- FLUID-4330
- Type
- Improvement
- Priority
- Major
- Status
- Closed
- Resolution
- Fixed
- Assignee
- Justin Obara
- Reporter
- Antranig Basman
- Created
2011-07-07T15:47:32.062-0400 - Updated
2017-02-22T21:06:14.931-0500 - Versions
-
- 1.4
- Fixed Versions
-
- 1.5
- Component
-
- IoC System
Description
Right now, many limitations exist on the relative timings of operations performed by components and subcomponents, and this restricts the kinds of configuration which we are able to offer for components. A case was encountered recently in UIOptions where some material, part of the "resources" block of the component, had a requirement on configuration delivered by a subcomponent, a URL template prefix. Since "resources" has direct value semantics and is represented by an "expander", it was impossible to grant access to the prefix value in resources from the component holding the prefix.
Other problems in this area - the "one demand block active per component" rule prevents such references being rebased by users, unless the material has its own DEDICATED component.
This problem will be resolved by the "globally ginger world", wherein options merging and IoC-driven subcomponent instantiation will proceed by a UNIFIED AND SYNCHRONOUS PROCESS.
Comments
-
Cindy Li commented
2011-07-07T15:55:23.436-0400 The real-world example of this issue is @ https://github.com/cindyli/infusion/tree/FLUID-4330
-
Antranig Basman commented
2012-08-29T20:36:55.670-0400 Further thoughts from a chat this morning
(10:12:54) AntranigBasman@googlemail.com/Gaim: It dawned on me this morning how I might make the GINGER WORLD work out.....
(10:13:02) AntranigBasman@googlemail.com/Gaim: Last night I went to bed thinking that it might be impossible : |
(10:13:13) michelled33@gmail.com: how will you make it work out?
(10:13:32) AntranigBasman@googlemail.com/Gaim: It's going to be more complicated than I had been thinking.... for a number of reasons
(10:13:56) AntranigBasman@googlemail.com/Gaim: Mainly on the performance angle... I went to bed thinking that I might, even after all this time, done exactly the wrong thing in abolishing trundlers
(10:14:06) AntranigBasman@googlemail.com/Gaim: Since all of a sudden I couldn't think of a way of making anything work without them
(10:14:19) AntranigBasman@googlemail.com/Gaim: But on the other hand, it was clear if I made anything work WITH them, the performance of it would be abysmal
(10:15:16) michelled33@gmail.com: have you found a way to not use them?
(10:15:19) AntranigBasman@googlemail.com/Gaim: This morning I realised that we need to have a kind of "bimodal" approach.... which is going to be a bit messy
(10:15:44) AntranigBasman@googlemail.com/Gaim: The issue is that all of the "expensive" algorithms have some CONTEXT, which expresses where they have got up to, in the merging or expanding process
(10:15:58) AntranigBasman@googlemail.com/Gaim: For example, they have some collection of objects in their hands, some arguments, some indexes, etc.
(10:16:10) AntranigBasman@googlemail.com/Gaim: In the old model, all of that context would go into the trundlers
(10:16:35) AntranigBasman@googlemail.com/Gaim: And now we fixed it so that all of the model access process is "context-free"
(10:17:26) AntranigBasman@googlemail.com/Gaim: But this now promises to make all of our "expensive algorithms" hugely more expensive for another reason.... that if they are expressed in terms of the new API, they will have to spend all of their time navigating back up from the bottom of all their object trees and reaquiring their context again
(10:18:04) AntranigBasman@googlemail.com/Gaim: So, I realised that we need to "split up" the workflow of these algorithms into what you could call "normal" and "abnormal" parts
(10:18:27) michelled33@gmail.com: what falls into each category?
(10:18:39) AntranigBasman@googlemail.com/Gaim: The "normal" parts look pretty much like the current algorithms, except that they will factor through an API that looks pretty much like the current strategy API, only it will have some EXTRA HIDDEN/SECRET arguments
(10:19:09) AntranigBasman@googlemail.com/Gaim: And these "normal" parts correspond to the normal "top to bottom" workflow that these algorithms have, that they start at the top of some tree of objects, and work their way to the bottom in the normal iteration order
(10:20:03) AntranigBasman@googlemail.com/Gaim: Now, the whole point of the GINGER WORLD stuff is that at various points, these algorithms may encounter material that forces them to start executing something "out of order" - for example, a demand for some either visited or unvisited part of their own options, or that of a different component which is itself only halfway through expanding
(10:21:24) AntranigBasman@googlemail.com/Gaim: So I think the only workable idea is that these parts of "abnormal workflow" will then pass through the standard context-free "strategy API" that we have - which need to somehow resolve onto the "same implementation" as themselves... but how this will be represented is by calling onto the same API we mentioned above, but with the EXTRA HIDDEN/SECRET ARGUMENTS all set to empty/undefined
(10:22:08) AntranigBasman@googlemail.com/Gaim: Which on spotting this, the algorithm will attempt to rediscover its context again, which will be moderately expensive, but will be sort of "pay as you go" in the sense that we will only do it when we explicitly discover a demand for some "out of order" material
(10:22:41) AntranigBasman@googlemail.com/Gaim: Which means if there is none of this stuff (as there currently is in our system, since it doesn't support it), expansion won't cost much more than it does now - just more by 1 function call plus a test for the "hidden arguments"
(10:23:11) AntranigBasman@googlemail.com/Gaim: "trundlers" seemed attractive since they seemed like a great place to stash this "extra hidden/secret" information as we went along.... but the big problem with them is that they incurred a huge allocation cost
(10:23:53) AntranigBasman@googlemail.com/Gaim: We could never be quite sure when we needed a new one, and we would need to keep allocating new ones constantly to make sure that noone got confused about whether they were sharing the same "cursor" as anyone else or not
(10:23:59) michelled33@gmail.com: ah, where were you seeing the issue with alllocation? cspace?
(10:24:03) AntranigBasman@googlemail.com/Gaim: Yes
(10:24:17) AntranigBasman@googlemail.com/Gaim: We discovered the CSpace's memory was perhaps 50% full of trundlers during page load
(10:24:24) AntranigBasman@googlemail.com/Gaim: Which is why we "threw them out" of the default workflow
(10:24:29) AntranigBasman@googlemail.com/Gaim: And now, threw them out entirely
(10:25:01) michelled33@gmail.com: well this all seems very sensible
(10:25:04) AntranigBasman@googlemail.com/Gaim: In my FLUID-4705 branch is a reimplementation of all of that workflow without them, where the cost of the "slow access system" isn't very much worse than the "fast access system" because now neither of them generates any garbage at all
(10:26:03) AntranigBasman@googlemail.com/Gaim: I added some comments to that JIRA with my measurements from yesterday...
(10:26:04) michelled33@gmail.com: oh, you've already implemented this
(10:26:11) AntranigBasman@googlemail.com/Gaim: Yes, trundlers are now ABOLISHED
(10:26:21) AntranigBasman@googlemail.com/Gaim: But the other part isn't implemented... the "bimodal system"
(10:26:27) AntranigBasman@googlemail.com/Gaim: Since I just thought of it about half an hour ago : P
(10:26:33) michelled33@gmail.com: 🙂
(10:26:45) AntranigBasman@googlemail.com/Gaim: But it will make the implementation of these "expensive algorithms" even more confusing than they used to be
(10:26:55) AntranigBasman@googlemail.com/Gaim: At each point where they do any work, there will now be a function call
(10:27:20) AntranigBasman@googlemail.com/Gaim: To something which superficially looks like the "strategy API" but which may or may not have some extra arguments, depending on whether the algorithm has remembered its context or not
(10:27:45) AntranigBasman@googlemail.com/Gaim: My idea right until today was that the "normal workflow" would be driven directly by the strategy API, which would mean that it looked the same for every algorithm
(10:28:04) AntranigBasman@googlemail.com/Gaim: That is, "traverse every property of this object recursively until you have seen them all, and call the strategy API for each one"
(10:28:18) AntranigBasman@googlemail.com/Gaim: But I realised last night that that would ALSO lead to completely unacceptable performance
(10:29:01) AntranigBasman@googlemail.com/Gaim: In fluid.merge, for example, it would spend all of its time traversing every merge argument starting from the root again, up to wherever it got to
(10:29:39) AntranigBasman@googlemail.com/Gaim: So it's clear that the "fast/normal algorithm" needs to look (almost) exactly like the current one, only with a function call written in the middle where it actually does some work
(10:29:58) AntranigBasman@googlemail.com/Gaim: I guess the HIDDEN/SECRET ARGUMENT could just consist of something very like the "insides of an old trundler"
(10:30:25) AntranigBasman@googlemail.com/Gaim: And either the algorithm is capable of coming up with it, because it has it in its hands due to the "normal workflow", or else it can't, because it is executing "out of order" and needs to come up with it all again
(10:30:57) AntranigBasman@googlemail.com/Gaim: but either way, the API looks the same
(10:31:21) AntranigBasman@googlemail.com/Gaim: There will be some duplication of function, since there will be two ways of recovering your context... or rather two different code paths it could come from
(10:31:41) AntranigBasman@googlemail.com/Gaim: Either you have your context "incrementally" since you had just stepped along to the next object in the traversal
(10:32:01) AntranigBasman@googlemail.com/Gaim: Or you had your context "from scratch" since you needed to start walking from the bottom of the tree again to find it....
(10:32:27) AntranigBasman@googlemail.com/Gaim: Depending on what that code actually looks like, it might be possible to factor out some common implementation, at the cost of yet another function call
(10:32:39) AntranigBasman@googlemail.com/Gaim: Despite the many years of progress, it seems that functions calls are still moderately expensive in FF 😞
(10:35:05) michelled33@gmail.com: ok, now I'm a little confused
(10:35:31) michelled33@gmail.com: if the secret args contain the stuff an on old trundler, where do we gain in performance?
(10:35:58) AntranigBasman@googlemail.com/Gaim: Well, the big problem with "old trundlers" is that they got constructed afresh every time we fetched something from the "results"
(10:36:05) AntranigBasman@googlemail.com/Gaim: That is, on every call to fluid.get or fluid.set
(10:36:29) michelled33@gmail.com: and now we would hold onto them?
(10:36:36) michelled33@gmail.com: or their equivalent?
(10:36:45) AntranigBasman@googlemail.com/Gaim: Well, now the algorithm itself either holds on to them, or forgets them, depending on where it has got up to
(10:37:14) AntranigBasman@googlemail.com/Gaim: The big problem with the old API was that you had to be conservative... you couldn't be sure why someone was calling fluid.get or set, so you had to make sure you started again
(10:37:14) michelled33@gmail.com: and if it's forgotten, then on a fluid.get or fluid.set they would get lazily created?
(10:37:39) AntranigBasman@googlemail.com/Gaim: And if EVERYTHING was implemented using get/set, even access to members of objects that were right under your hands, you would need to pay all the costs of constructing the trundler from scratch again
(10:38:01) michelled33@gmail.com: yes, that seems crazy
(10:38:15) AntranigBasman@googlemail.com/Gaim: Whereas in practice, just in the same way the the loops etc. in the current "normal" algorithms were written, if you happened to KNOW what you were up to was just "considering the next object in order" you could come up with all the contents of them very cheaply
(10:38:57) AntranigBasman@googlemail.com/Gaim: In practice, if you are just moving on to the next sibling of the same parent, all you need to do is update a variable holding a property name
(10:39:13) AntranigBasman@googlemail.com/Gaim: Or, if you are moving on to a new child, there is a more expensive operation where you might update a bunch of objects
(10:39:30) AntranigBasman@googlemail.com/Gaim: But still, these incremental costs CAN be very small, assuming you are not moving along too far in the tree
(10:39:56) AntranigBasman@googlemail.com/Gaim: But the big issue with GINGERNESS is that it might cause you to have to switch over to implementing... essentially the SAME algorithm, but perhaps at a completely different place in the tree
(10:40:12) AntranigBasman@googlemail.com/Gaim: But you can never know where or when it will be, since it is "data driven" - either the thing under your hands is an EL reference to yourself, or it isn't
(10:40:38) AntranigBasman@googlemail.com/Gaim: So there's nothing sensible you can do about storing "restart objects" around in the tree to help you to remember what you need to know, because you would need to store them EVERYWHERE
(10:40:44) AntranigBasman@googlemail.com/Gaim: Which implies that there would be garbage everywhere -
Antranig Basman commented
2012-09-01T01:14:38.539-0400 Further talk from this morning:
(08:22:49) AntranigBasman: http://pastie.org/4636446
(08:22:58) AntranigBasman: I have been staring at it all night, very slowly
(08:23:05) AntranigBasman: It's certainly a good deal less simple than it was
(08:23:25) AntranigBasman: Here are the old and new versions of the same implementation
(08:23:31) AntranigBasman: It doesn't even do all that it used to, yet
(08:23:32) Hens Respectable: ah
(08:23:52) AntranigBasman: But I imagine that it will be possible to economise on a lot of the implementation, once we have more than one ZWITTERIONIC thing
(08:24:06) AntranigBasman: The main ZWITTERARGUMENTATIVE ENTRY POINT is on line 10
(08:24:26) AntranigBasman: As I planned, the arguments exist in two forms - EITHER "i" and "segs" are set, OR "source" is set
(08:24:43) AntranigBasman: It's all still a bit peculiar
(08:24:47) Hens Respectable: you still have to pass something in though right?
(08:24:53) Hens Respectable: undefined or null?
(08:25:06) AntranigBasman: I am still trying to work out whether we really can preserve the "noCopy" mode
(08:25:18) AntranigBasman: It's pretty awkward you can't return more than one value at a time in JS
(08:25:43) AntranigBasman: Although I'm starting to wonder whether the VMs really do strongly optimise the case of returning an object which is then GCed immediately
(08:25:51) AntranigBasman: I should do a little test...
(08:26:22) AntranigBasman: Since this whole "diagnoseTrunk" business is designed to deal with not producing extra garbage
(08:27:30) Hens Respectable: well, I mean, if you had some sort of named arguments, the signature would probably be easier to deal with too
(08:27:43) AntranigBasman: Right
(08:27:46) Hens Respectable: sicne you still have to fill though in even if you're not using them
(08:27:50) AntranigBasman: That would increase the expense a lot too
(08:27:55) AntranigBasman: Since we would constantly be doing property access
(08:28:01) AntranigBasman: Which is definitely more expensive, even in V8
(08:28:29) Hens Respectable: have you run this yet, or are you still designing it?
(08:28:34) AntranigBasman: It doesn't run
(08:28:41) AntranigBasman: It is still "code for staring at"
(08:28:47) Hens Respectable: 🙂
(08:29:22) AntranigBasman: Supporting the "filter" property is going to be the first hard thing
(08:29:25) AntranigBasman: I just woke up thinking about it
(08:29:35) AntranigBasman: Since it seems that that code needs to fork into two different places in the algorithm
(08:29:44) AntranigBasman: part of it needs to go into "regenerateCursor"
(08:29:54) AntranigBasman: And I started to think about whether the two pieces of workflow couldn't be combined after all
(08:30:14) AntranigBasman: Line 12 is already a bit dubious
(08:30:20) AntranigBasman: Since in theory this is part of someone else's workflow
(08:30:52) AntranigBasman: Notice the rather funky functional self-reference, from line 27 to line 10
(08:31:32) AntranigBasman: It is EVEN MORE DIRECT functional recursion than before, since the function needs to refer to itself before it's even been defined
(08:31:41) AntranigBasman: The kind of thing that would tie up a C++ interpreter in knots
(08:32:05) Hens Respectable: hahahahha
(08:32:17) Hens Respectable: what is isTrunk for?
(08:32:40) AntranigBasman: But the direct function recursion is really pretty funky since it knows that it is calling itself, so it doesn't even need to bother to fill in 2 of its arguments if it knows it isn't going to use them
(08:32:41) AntranigBasman: Quite bizarre
(08:32:55) AntranigBasman: isTrunk is used to determine whether any recursion is going to happen or not
(08:33:12) AntranigBasman: If it is not set, the "left hand side" is considered complete at this round
(08:33:36) Hens Respectable: ah
(08:33:37) AntranigBasman: The problem is that the return value for disposeSource needs to do 2 things
(08:33:54) AntranigBasman: i) it may be an object of ANY type, and ii) quite separately, it might signal whether recursion needs to happen or not
(08:34:18) AntranigBasman: This all used to be possible quite cleanly in the old body of "resolveEnvironmentImpl" since you could just happily keep on cascading an if/else statement
(08:35:05) AntranigBasman: I guess in theory I could keep on writing out more of the body back home in makeExpandStrategy, but I am expecting that this "trunk code" will become a shared utility
(08:35:15) AntranigBasman: And that lines 21-29 will to some extent become sharable
(08:35:19) Hens Respectable: why is that called disposeSource? It appears that it's returning an object if it's not in use
(08:35:52) AntranigBasman: It means "dispose" as in "figure out what to do with"
(08:35:58) Hens Respectable: ah
(08:36:05) Hens Respectable: like in the princess bride
(08:36:11) AntranigBasman: As in "Man proposes, God disposes" : P
(08:37:00) AntranigBasman: It doesn't NECESSARILY mean that God may choose to destroy you, although in practice that may be what in fact ends up happening : P
(08:38:10) Hens Respectable: HAHAHHAHAHAHAHA
(08:38:44) AntranigBasman: I guess in practice nothing after line 18 is reusable
(08:38:52) AntranigBasman: So I may as well just write out the body of disposeSource inline
(08:39:32) AntranigBasman: Which will indeed get rid of all this silly "trunk" business and let us just use if/else again
(08:39:55) AntranigBasman: We only have three implementations in the entire world in any case
(08:40:09) AntranigBasman: i) this thing - expansion, ii) merging, iii) model transformation
(08:40:24) Hens Respectable: are you sure there might not be more?
(08:40:31) AntranigBasman: Well, there never have been more
(08:40:33) AntranigBasman: Until now
(08:40:53) AntranigBasman: Every kind of expansion that we have currently bottles through this one implementation
(08:41:01) AntranigBasman: So it is already quite "economised" to that extent
(08:41:26) AntranigBasman: Even renderer component trees use it, although we don't actually need any kind of "gingerness" for those
(08:41:44) AntranigBasman: It's impossible to issue references into the middle of a renderer tree, so you may as well just expand them all at once
(08:43:30) AntranigBasman: Line 17 needs to be rewritten to redispatch to the strategy API once again
(08:43:44) AntranigBasman: Since we may be consuming a structure that is similarly "lazily produced" as we are
(08:43:55) AntranigBasman: So we can't just blindly go accessing properties directly
(08:44:33) AntranigBasman: Luckily we have all of the first 4 arguments tidily sitting there to send it........ assuming they are still in step........
(08:45:08) AntranigBasman: The whole business of "filter" is where the shit starts hitting the fan.... since if there are expanders in the way, the two trees stop being isomorphic
(08:45:22) AntranigBasman: There may be any number of levels of expanders which cause them to be misaligned
(08:45:37) AntranigBasman: And you have to be able to deal with this both in the "direct" and "indirect" dispatch modes
(08:45:43) AntranigBasman: Which is what I was thinking about this morning
(08:46:16) AntranigBasman: It seems crazy to write out the whole "expander" workflow twice, but WHERE on earth can you write it out just once.....
(08:46:51) AntranigBasman: AS SOON as you ever discover an expander, you are immediately going to proceed to try to expand it immediately
(08:47:14) AntranigBasman: Which seems to imply you could never legitimately encounter it a 2nd time.... but I guess you really could
(08:47:31) AntranigBasman: You could discover, within the expanded material, a redispatch into ANOTHER location within the same expanded material before you have finished expanding it
(08:47:39) AntranigBasman: ...........................
(08:48:30) AntranigBasman: This stuff ends up polluting all the APIs of everything all the way up
(08:48:44) AntranigBasman: Not only does the "filter" API get ripped up, the "expander" one does too
(08:48:56) AntranigBasman: And that's one of the ones that's been stabilised for the longest time
(08:49:55) AntranigBasman: No sooner did I design the "strategy" API this week, but it ends up becoming the only new universally stable API...................
(08:50:12) AntranigBasman: I knew somehow this would be "strategic" : P
(08:50:24) AntranigBasman: That I couldn't embark on any of this work until I knew what trundlers would be replaced by.....
(08:50:58) AntranigBasman: "expanders" used to be so blissfully simple, since it was assumed that they could just return values any old how
(08:51:14) AntranigBasman: The inability to simply write return values is what we've lost in all of this
(08:51:26) AntranigBasman: It becomes impossible to talk about a value without having to say EXACTLY WHERE it is going to go
(08:51:38) AntranigBasman: And exactly what EL path it is going to have, before you even produce it
(08:52:05) AntranigBasman: You aren't allowed to produce a value, unless someone has already issued its EL path ALREADY
(08:52:07) Hens Respectable: Where it is going to go in the lifecycle, or where it's gettring returned to? ( middle of a traversal etc)
(08:53:57) AntranigBasman: Ironically, we only support about 2 expanders here anyway
(08:54:00) AntranigBasman: And one of them is very silly
(08:54:05) AntranigBasman: And the other one should be unncessary
(08:54:59) AntranigBasman: Although I guess they are both interesting, in their way....
(08:55:22) AntranigBasman: We have the "deferred fetch" expander.... which fetches something via AJAX and replaces it in here... at some later point
(08:55:37) AntranigBasman: And then the "noexpand" expander, which simply dumps its argument here and terminates the expansion process
(08:56:16) AntranigBasman: Implementing the latter is very easy, and implementing the former suddenly seems extremely hard
(08:56:37) AntranigBasman: Awful, in fact.... anyone who issues a demand for any of the fetched values will need to be made to block
(08:59:16) AntranigBasman: Which implies that the self-dispatch here, on line 17, and indeed everywhere else, needs to be wrapped in a callback and potentially receive a "promise"
(08:59:42) AntranigBasman: Everything which ever tries to read a value needs to prepare for the possibility it is not here yet
(09:00:03) AntranigBasman: I guess this is what we joined the army for, but it seems a bit painful to have to implement it immediately
(09:02:22) AntranigBasman: Potential costs of having all these closures around also seems horrific............................
(09:02:34) Hens Respectable: hahahahaand
14:19:55) AntranigBasman: It is still bending my brain enormously, even in this simple case
(14:20:11) AntranigBasman: I guess I should commit this slightly crazed implementation and our conversation for the records.....
(14:20:40) AntranigBasman: All of the material above is the replacement for the old resolveEnvironmentImpl.... and it's not even slightly complete yet
(14:21:46) AntranigBasman: It can't deal with expanders or noCopy............
(14:22:38) Isle of Yura: oh
(14:23:29) AntranigBasman: I've been boggling about expanders all day.... in and out of bouts of unconsciousness : P
(14:24:05) AntranigBasman: Seems like the best way to deal with them would be to have up to THREE trees.... although it may still be possible to recover "noCopy" in the end and make them the same tree in some cases
(14:24:16) AntranigBasman: Well, no.... there can be at a minimum two
(14:24:26) AntranigBasman: Since you can't commit stuff to the left tree that isn't absolutely final
(14:24:44) AntranigBasman: So, up to 3 trees, "target", "aux" and "source"
(14:24:54) AntranigBasman: "target" contains only things that are absolutely complete
(14:25:08) AntranigBasman: "aux" contains a modified version of "source" accounting for the partial state of expanders and the like
(14:25:34) AntranigBasman: "source" may or may not be the same as "aux" depending on whether you wanted "noCopy" or not and consented to having the input tree corrupted
(14:26:24) AntranigBasman: Probably the savings in "noCopy" will be negligible in the end... although the implementation comment suggests that that was really about model preservation rather than efficiency
(14:26:43) AntranigBasman: Which in the long run we would hope to be able to deal with by SEEING INTO TIME and seeing that we're actually expanding something that has an aligned mergePolicy attached to it
(14:27:46) Isle of Yura: i feel lightheaded now
(14:27:47) Isle of Yura: 🙂
(14:39:05) AntranigBasman: I guess it's not unexpectedly entirely unlike the git model....
(14:39:18) AntranigBasman: You could think of "target" as the repository, "source" is the working copy, and "aux" is the index.................. -
Antranig Basman commented
2012-09-01T01:18:26.306-0400 Further thoughts: We simply HAVE to revive some kind of trundler-like functionality - although the "zwitterargument" idea is (basically?) correct, the problem is at line 17 where we really need access to the "dependent strategy" which should be able to economise from being trundled along with us:
var thisSource = source[name];
But then, assuming we have this operation, wouldn't it in fact then be the most efficient way to implement "regenerateCursor" outside, in the framework? The most value we seem to have got from abolishing "old trundlers" was simply to save on a closure, a bunch of function handles and a complex object. Should do some performance investigation to see if "cheap structures" really are scavenged effectively by the VM, as our inability to improve on the performance of the "old fast" fluid.get seems to suggest.
-
Antranig Basman commented
2012-09-04T03:14:34.344-0400 Current ideas: Implementing "simultaneous expansion and merging" was looking to be quite a challenge, and it seemed extremely inviting to see whether we really needed it. As an experiment, fluid.expandComponentOptions was hacked in a very basic way to assemble all merge arguments first, and then defer to expansion later after merging. Amazingly, after a bit of tinkering to allow resolving simple strings like "{options}" onto the "local record", this managed to get 100% of the test cases in FluidIoCTests.js passing, other than the "advanced circularity test" which now failed through failing to throw an exception, which was quite a minor loss.
However, proceeding to the wider framework, the following case from UIEnhancer.js was quickly found:
fluid.defaults("fluid.pageEnhancer", { gradeNames: ["fluid.littleComponent"], components: { uiEnhancer: { type: "fluid.uiEnhancer", container: "body", options: "{pageEnhancer}.uiEnhancerOptions" } } });
Unfortunately although this use of the framework here is an egregious hack to get around precisely the merging issues we are trying to fix, it doesn't really seem like this case can be forbidden. There seems to be no satisfactory way to represent this record as a merge argument - given it is a string, it could never be merged, and if it was expanded, it obviously requires "true expansion".
Other, more harmless cases look like this:
textFont: { type: "fluid.uiEnhancer.classSwapper", container: "{uiEnhancer}.container", options: { classes: "{uiEnhancer}.options.classnameMap.textFont" } },
In general, it seemed that because mergePolicies are now fairly robust in the face of things like listeners and events, we could have got away with a "merge-first" model but ultimately it seems like we can't. We can at least (possibly) make the model more regular, since the various calls to expandOptions within expandComponentOptions used a random mishmash of arguments - in general there doesn't seem to be any reason why everything can't be expanded, but the "local record" is quite a problem - given the meaning of values such as "{options}" is highly context-dependent. Note that FLUID-4631 is a case reflecting confusion of this form, in this case the meaning of "arguments". In general a reference to fully-qualified component-relative expressions (or indeed "{that}" which is a placeholder for same) should only refer to FULLY EXPANDED AND MERGED values - however, references to arguments or pseudoarguments such as "{options}", "{arguments}", "{container}" and the like clearly need to operate different rules, most likely referring to "the record which was most recent in merge order". Further confusion can then arise when these refer to material which then needs to be expanded AGAIN.
Clearly use of the pseudoarguments must be forbidden to be mixed with fully qualified references to head off that confusion - unfortunately the current implementation of fluid.expandOptions doesn't deal with any such separation. Lots of the mess relating to "EXPAND", "EXPAND_NOW" and "{directOptions}" is clearly an ad hoc system targetted at this kind of confusion. The case listed above of options: "{pageEnhancer}.uiEnhancerOptions" is a prime case. Clearly it might have also been possible to write something a bit perverse like options: "{options}.subpath" in this case - and it's clear that the former case delivers REAL options which have already been expanded, but the latter case presumably delivers options which require expanding again. To knowledge, noone has written anything as perverse as "{options}.subpath" but on the other hand they do write things like
fluid.demands("boiledLocal", "fluid.tests.eventChild", [ "{arguments}.0", "{eventChild}" ]);
In the case of non-options material there seems to be much less scope for confusion since no merging is (currently) happening - if only because we have no support for multiple demands blocks referring to the same material. In this case both styles of reference clearly resolve onto concrete objects (since {arguments} can only refer to the REAL user's concrete arguments which were just fired) and there is no ambiguity about i) which "level of reference in the local record" is occurring (there is only one level) and ii) whether a further round of expansion is required (it never is).
Upshot so far:
a) more test cases are required in FluidIoC to deal with previously untested cases which do indeed require "expansion before merging" and
b) it seems that simultaneous expansion and merging is indeed required, which will be quite a performance and implementation risk, as well as involving
c) apparently supplying each "expansion point" with its own "localRecord". In theory anything resolved from the localRecord can't cause a circularity risk since it is sourced directly from configuration rather than "live values", but on the other hand presumably does require EXPANDING AGAIN precisely because of this, at which point it becomes live.Here is an example of precisely this perverse case: UIOptions.js line 907:
fluid.demands("fluid.uiEnhancer", "fluid.uiOptions.preview", { funcName: "fluid.uiEnhancer", args: [ "{preview}.enhancerContainer", "{options}" ] });
This presumably should be eliminated since it could be expressed just in terms of an overriding of "{container}" and dates from the pre-pseudoargument days.
Here however is a worse example from the Uploader:
fluid.demands("fluid.uploader.remote", "fluid.uploader.swfUploadStrategy", { funcName: "fluid.uploader.swfUploadStrategy.remote", args: [ "{engine}.swfUpload", "{multiFileUploader}.queue", "{options}" ] });
"remote" is indeed a genuine littleComponent, using the favorite device of Colin of a non-standard signature. The meaning of the demands block here is only clear because there can only be ONE of it - and the reference of "options" is relatively clear, since presumably it refers to material held in defaults or else a subcomponent record. However, the behaviour if multiple demands blocks were to attempt to influence this pathway would be unclear - although perhaps what would be desirable is if one of them could be definitively LAST, in which case "{options}" would clearly refer to "the accumulated merged and expanded options to date", and all of the others were expressed purely in terms of options: on the left hand side. Currently we (I believe) have no support for merging together general argument lists. For a start, there would be nowhere to write the relevant merge policies.
These two cases (UIOptions, Uploader) are the only forms where "{options}" appears in user code in the framework. However, the framework itself issues "{options}" itself sometimes, especially in the (strongly doomed) "mergeAllOptions" pathway as well as substituting for the old fluid.COMPONENT_OPTIONS value. The latter is used in the Uploader in some more places but is currently disused... I have vague memories of it being more popular once. From the mailing list I see this exchange:
http://lists.idrc.ocad.ca/pipermail/fluid-work/2009-June/004665.html
Referring to a VERY old model for instantiation (this was before any kind of IoC was implemented as working) as follows:
that.pageList = fluid.initSubcomponent(that, "pageList", [container, events, that.options, fluid.COMPONENT_OPTIONS, strings]);
In this case, the placeholder was meant to refer to whatever material was in the subcomponent record - this is broadly consistent with the uses above.
In theory we would simply like to forbid all uses of "{options}" entirely. In the future framework, it will be clear that EVERY contribution will be merged against all previous ones (e.g. contributed from demands blocks), and in theory, its use in the Uploader demands blocks above is redundant because the ferrying of options to the correct argument position should just be inferred from the component GRADE.
It seems that all outstanding uses of "{options}" are of this form - raw argument ferrying - and so the need to support perversities like "{options}.subPath}" or indeed "{options}" at all should be declared nonexistent. If all "pseudoargument ferrying" is instead done by GRADES then we are left with only legitimate (absolute) references which can occur in configuration This hopefully removes this ambiguity - every reference which is then encountered during expansion is thus one of these legitimate references which will then require no further expansion - so long as we can remove the framework's own use of these pseudoexpressions - at least this latter can be controlled to prevent the possibility of structures which contain mixed styles. This assumption was really what underlay the "experimental code" written this evening of the following form:
fluid.expandLocal = function(value, localRecord) { if (!value) { return value; } if (typeof(value) !== "string") { return value; } var parsed = fluid.parseContextReference(value, 0); return fluid.get(fluid.get(localRecord, parsed.context), parsed.path); };
But indeed as it turned out this is just too simple to be effective. The use case we came in with, - options: "{pageEnhancer}.uiEnhancerOptions" - has to be considered bizarre though legitimate.
-
Antranig Basman commented
2012-09-04T03:39:51.354-0400 Disposition - experimental work committed to my repo at 5e4dd65. The further reasoning then invalidates point c) - no ferrying of "localRecords" is necessary since there is only possibly one for an entire instantiation/resolution. This does probably mean we should just commit a straightforward fix for Justin's FLUID-4631 at least.
This ignores any role for insertion of any model transformation in the pipelines. These will indeed require references to "the most recent expansion/merging product so far" rather than only global references. This also implies some kind of "right to left" model - values contributed by the "end user" (as arguments, or even sometimes in component records and demands blocks) are exactly those which require transforming whereas obviously our own defaults, with the weakest merge priority, issued in our own code, are already correct.
Now worry about how enormously expensive the merge algorithm may become - it seems it has to remain pairwise as before, but needs to deal with the case where the "driver" on the RHS is either concrete or not, or maybe, in some future horrific case, is actually asynchronous. We then need to start working on this "reconstructCursor" business since clearly the place where there are the greatest unrealised savings are in the continued "locality of reference" wrt. RHS objects. Clearly when we blindly issue a reference into the "global world" there are not going to be any savings to be realised, but hopefully this "reconstructCursor" can be reimplemented, and also automatically implemented, in terms of incremental and cheap operations on something like a "trundler". To really work well this requires strong "homogeneity" conditions on the RHS object, which in real life I guess will indeed be satisfied - in the "expansion and merging pathway" in all the cases where we require to exploit locality of reference we ALSO know that we have "homogeneous trundling". It is only the expensive, nonlocal "global reference" model where we have a heterogeneous trundler, and don't mind.
-
Antranig Basman commented
2012-09-04T04:17:22.267-0400 By using the "EXPANSION IS REIFICATION" model that we have in the existing trial implementation, we can probably economise on a lot of the logic in mergeImpl - we only need to take some special action when we discover that some property is undefined..... although of course, the first time round, which is the only time round, this will be all the time ...... !
No point implementing any kind of "read-ahead" on the RHS assuming we can realise any locality benefits anyway... so this just lumbers us back with the original problem we had with trundlers right at the start... the "argument update problem", as well as how to keep a set of "cursors" which can unset themselves when popping off a stack frame without filling memory with garbage. The "cursor is JUST an object reference" is the only really affordable one, which points back to vindicating the original redesign of trundlers and the ZWITTERARGUMENT model. Even the sharedness of "segs" isn't completely unproblematic though. In theory, if we are in the "local cursor" model the driver on the RHS doesn't care about segs either though, assuming we update its cursor correctly.Upshot: We can't really save on a couple of extra function calls every time round mergeImpl, but given that bizarrely "mergePolicyIs" didn't seem to penalise us much, these should be ok as long as they don't create extra closures. WHY ON EARTH isn't the new fluid.get faster than the old one!! Need to check again whether it is purely result of long argument lists one of which is a function pointer. New mergeImpl will be very similar. In theory we can just gives its RHS some more arguments ("trundler"/cursor updator) so that we can share as much as possible with manual users of fluid.merge - although it's worth noting that NONE of these users should ever be merging component options so in theory we could just split off a very simple jQuery.extend-like core to leave behind for those operators on simple concrete structures.
-
Antranig Basman commented
2013-02-19T14:55:08.308-0500 Resolved by merge of FLUID-4330 branch at revision 91d5d1