Adaptation of @tsing80’s node joining merge tree #43
Conversation
stefanpenner
force-pushed
the
join-nodes
branch
from
4b8d718 to
6a355df
Compare
index.js
Outdated
| @@ -38,6 +40,28 @@ function BroccoliMergeTrees(inputNodes, options) { | |||
| this._currentTree = FSTree.fromPaths([]); | |||
| } | |||
|
|
|||
| function joinNodes(host, options, inputNodes) { | |||
There was a problem hiding this comment.
needs unit test
|
How badly do we still want this change? While the graph after applying this change looks visually simpler, I wonder if it might actually be harder to follow. Merging trees could be extremely cheap in an ideal world, so I would perhaps hope that the need for this might go away as we apply more and more FSTree optimizations. |
|
@rwjblue we should chat about this, I know you have a wrapper that does this. Should be upstream? |
index.js
Outdated
| function BroccoliMergeTrees(inputNodes, options) { | ||
| if (!(this instanceof BroccoliMergeTrees)) return new BroccoliMergeTrees(inputNodes, options) | ||
| options = options || {} | ||
| var name = 'broccoli-merge-trees:' + (options.annotation || '') | ||
| if (!Array.isArray(inputNodes)) { | ||
| throw new TypeError(name + ': Expected array, got: [' + inputNodes +']') | ||
| } | ||
| Plugin.call(this, inputNodes, { | ||
| Plugin.call(this, joinNodes(this, options, inputNodes), { |
index.js
Outdated
| @@ -15,21 +15,23 @@ function unlinkOrRmrfSync(path) { | |||
| rimraf.sync(path); | |||
| } | |||
| } | |||
| var ASSIMILATION_SYMBOL = 'broccoli-merge-tree@1.0.0'; | |||
There was a problem hiding this comment.
doesn't need to be semver, a seperate ASSIMILATION protocol versions is likely best. Allowing for multiple version collapses..
stefanpenner
force-pushed
the
join-nodes
branch
from
6a355df to
d474145
Compare
—— Given an array of inputNodes, this function will attempt to inline all compatible MergeTree inputNodes (recursively) with the current MergeTree A child inputTree is considered inline safe it it: * has a matching options.overwrite * shares the same ASSIMILATION_PROTOCOL_VERSION This function will also drop obvious duplicates.
stefanpenner
force-pushed
the
join-nodes
branch
from
d474145 to
28835cc
Compare
|
@stefanpenner curious if this PR can be ressurected? What we've noticed in our monorepo, which has a lot of packages/apps, is that mergeTrees takes up to 60% of time to process, as well as 30% of time we spent on GCs from V8.
It seems like walkSync / mergeDirectories takes a lot of time. Also, as a side question - is there a possibility to minimize how much I am happy to dig around, but before doing that I'd appreciate if it will be possible to have a chat and gain some insights, what were the original problems and cases to look at :) eg. some windows sheneningans, or some other edge-cases where specifically covered in current implementation? Thanks in advance! |
takes:
and makes it:
this particular test has to much noise to verify the performance characteristics. I'll try to clean it up yet.
this checks in at about a 10-15% total build time improvement