Jermolene
Feature: Modular Attribute Rules & Selective Attribute Refresh
Implemented "modular attributes", comprising two new module-types: "attributerule" (analog to a wikirule) and "attributevalue" (analog to a widget). These allow plugins to define new attribute syntaxes and may open the door to some performance improvements.
I've implemented modules for parsing and executing filtered, indirect and macro attributes. The if/else chains in parseutils.js and widget.js have been replaced with hashmap lookups. The implementation here avoids re-processing string literals or re-checking types when re-computing attributes.
The attribute value modules implement an unused "refresh" method. In the future these could be used to reduce tiddler store accesses and macro expansions when the tree is refreshed.
See discussion on TiddlyWikiDev: https://groups.google.com/forum/#!topic/tiddlywikidev/VpKQquX33ts
For sake of clarity, my agenda with this pull request is to achieve core support for a custom attribute syntax in my formulas plugin:
<$text text=(= 15% * sum([tag[Income]get[value]]) - sum([tag[Expense]get[value]]) =) />
Fixed a bug in the PR when expanding variables that don't exist.
Issue: There appears to be a bug where transclusions of the form {{!!field}} don't work with this modification. Hard to say why. I notice that there are slight differences in the widget tree generated for these expressions with the modified code.
Fixed the aforementioned issue after pinning it down to string attributes with no actual value. The "undefined" value was being transcribed by computeAttributes.
I would like to discuss adding a recomputeAttributes(changedTiddlers) function to this PR.
Added a reference implementation of a refreshAttributes function, updated PR title to reflect the expanded feature.
IMO the ultimate test is: https://tiddlywiki.com/#%24%3A%2Fcore%2Fsave%2Fall
which seems to be about 3.5 seconds on FF 57 win10. ... Chrome has a problem on my system: it's between 15-30 seconds @Jermolene
Re: performance, in the long run I'm interested in making visual programs like this more responsive and "scalable":
https://evanbalster.com/tiddlywiki/formulas.html#Demo%3A%20Harmonic%20Lattice
Currently $formula-vars instigates a re-render whenever its outputs change, although at a glance it looks like most of the work involved may be unnecessary. I have another, fancier version of this in my dev wiki which adds a few more elements for each point, and this results in a large drop in framerate as the slider moves around (most of which is under FormulaVars.refreshChildren). This suggests to me that a lot of the marginal cost could be in the DOM rebuild. WikiText macro re-executions are particularly pricey, and have no benefit if the arguments haven't changed...
That "grand plan" certainly won't be fulfilled by attribute modules alone. The other big change will be introducing a "changedVariables" parameter to refresh methods throughout TiddlyWiki, which should allow a lot of unnecessary refreshSelf() calls to be avoided.
IMO the ultimate test is: https://tiddlywiki.com/#%24%3A%2Fcore%2Fsave%2Fall
So, my focus is actually more on improving refresh performance for interactive pages. Complex TiddlyWiki code gets a bit ponderous when the tiddler store is large, which incentivizes users to break large wikis up into smaller ones. I think that expanding selective refresh could make a huge impact in this department.
I did some testing with an expensive SVG DOM tree (and with DOM rebuilding artificially suppressed). The attribute modules implementation appears to be about 25-30% faster than the one in master, but performance gains are dwarfed by the cost of calls to setAttributeNS(). Most of the attributes in my test-case are variables and strings. I'm running Chrome on OS X.
I did some testing with an expensive SVG DOM tree (and with DOM rebuilding artificially suppressed). The attribute modules implementation appears to be about 25-30% faster than the one in master, but performance gains are dwarfed by the cost of calls to setAttributeNS(). Most of the attributes in my test-case are variables and strings. I'm running Chrome on OS X.
The "save/all" link wasn't intended as a performance test case. IMO it's a compatibility testcase, which seems to have a problem with Windows Chrome ...
You are messing around with low level core internals. So the most important part is compatibility! ... Did you run the TW tests, with your changes active?
Did just run TW-tests with your PR active. ... They all pass. Which actually doesn't mean too much, since we definitely should have more tests. ... But the basics seem to be ok.
@pmario Pardon my incompetence, but I haven't really figured out the build workflow for the TiddlyWiki5 repository. The empty edition build doesn't seem to incorporate my changes... So any guidance on that would be useful!
My rationale re: testing is that attributes are so ubiquitous that errors tend to catastrophically wreck TiddlyWiki, or otherwise cause trouble in complex widget code. That doesn't seem to happen now. :)
It also helps that the new code is fairly small and doesn't depart too far from the old logic. I'm counting on code review from you and @Jermolene to vet this stuff.
Addressed some of @PMario's code style remarks. Outstanding issues:
- Do
attributevaluemodels deserve a name befitting their similarity to widgets? - Where in the namespace should we store loaded attribute-value and attribute-parser modules?
... Pardon my incompetence, but I haven't really figured out the build workflow for the TiddlyWiki5 repository. The empty edition build doesn't seem to incorporate my changes... So any guidance on that would be useful!
Which text editor do you use atm?
I'll record a short video, which is much less work (for me) as describing the workflow with text.
I think you are producing great stuff. .. I like the formular library you introduced. ... It's sad, that the core seems to block you. ... But on the other hand, it gives us a real reason to have a closer look again ;)
attributevalue "classes" are now handled in a fashion more similar to widget classes.
It isn't yet clear how the same convention could be followed for parseAttribute. Seems like maybe the attributerule classes should live in WikiParser like WikiRules... But it's not conventional to pass a parser to parse utility functions.
A video on how to build the core of Tiddlywiki would be very helpful (would this be after forking it from Jermolene's github?).
I'm trying to figure out how to insert some "JsonPointer" (here https://www.npmjs.com/package/json-pointer) code that will help with accessing nested JSON objects in data tiddlers.
But I will totally admit that tiddywiki uses some architecture concepts I have not run into before, so I want to avoid "gotchas" (like a previously mentioned "don use require() in the core" note).
I will probably be using Evan's excellent work on Forumlas and his [[$:/plugins/ebalster/attribute-modules]] experiment as a model, but am not sure how to hook that library into the core.
Mahalo (thanks)!
On Sat, Dec 30, 2017 at 3:33 AM, Mario Pietsch [email protected] wrote:
... Pardon my incompetence, but I haven't really figured out the build workflow for the TiddlyWiki5 repository. The empty edition build doesn't seem to incorporate my changes... So any guidance on that would be useful!
Which text editor do you use atm?
I'll record a short video, which is much less work (for me) as describing the workflow with text.
I think you are producing great stuff. .. I like the formular library you introduced. ... It's sad, that the core seems to block you. ... But on the other hand, it gives us a real reason to have a closer look again ;)
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Jermolene/TiddlyWiki5/pull/3072#issuecomment-354541393, or mute the thread https://github.com/notifications/unsubscribe-auth/AIrmBzEZamtyq9aMXdbNeF7whkTI_SNbks5tFh-BgaJpZM4RNuoH .
---- OT ----
I'm trying to figure out how to insert some "JsonPointer" (here https://www.npmjs.com/package/json-pointer) code that will help with accessing nested JSON objects in data tiddlers.
IMO that's plugin territory. JSON means JavaScript Object Notation, which natively is part of JS already.
So every library, that implements a higher level API is created for a "specialized" usecase. ... TW core should be neutral. ... but that's slightly off topic here
@joshuafontany @pmario Maybe open a new topic to discuss this, guys.
FWIW, the TiddlyWiki documentation seems to suggest that access to non-root JSON variables was considered as a core feature...
https://tiddlywiki.com/#DataTiddlers Note: It is currently only possible to retrieve data from the immediate properties of the root object of a JSONTiddler.