-
Notifications
You must be signed in to change notification settings - Fork 91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Creating a snapshot causes pages to become unresponsive #25
Comments
@Mctalian great...thanks. The solution I've been thinking of involves a possible new js-lib. It's a small lib and will fasten scripts such as getSnapshot function or any other arbitrary "long-running" scripts. Initial and simple tests shows about 10 times faster execution. I'll get back as soon as there is anything to show... Meanwhile, I'll be testing this with your JSON on a feature-branch and will get back regarding this. |
@Mctalian here is status update; The samll lib I mentioned earlier is this one; This lib offers an easy interface to web workers and my idea is to create defiant snapshots using web workers. Using this lib have two effects;
This is the branch that I am testing this new feature on: |
@hbi99 That sounds awesome! Once it's ready for me to take a crack at it, let me know. I'm assuming by saying "it is not finished" that attempting to consume it in its current state wouldn't be best. So I'll wait for the green or yellow light to give it a try on my local build, which should take about 2 minutes to try that out. |
Hey @Mctalian, The file "json-search-6.htm" is loading your JSON and performs a search on a snapshot created with the new feature: Below is example of how it works...if the method getSnapshot second argument is a function, then the snapshot will be delivered to that function as soon as it's created. Defiant.getSnapshot(json_data, function(snapshot) {
// perform search
JSON.search(snapshot, 'xpath_expression');
}); To clarify, this code will not block the UI thread while the snapshot is being created. |
@hbi99 Ok, I'm running out of ideas. I was able to import my JSON data into your json-search-6 demo and it worked there. In my actual application, however I am not getting into the callback function. I place a console.log() right at the beginning and I see nothing. I can see that my CPU utilization is at a constant 25% during this time and at one point it rose up to 99% when I left it alone long enough. Here are some snippets of my code: function initialize(vals) {
root = angular.copy(vals);
if (scope.brotherId) {
// a snapshot is created here on special cases, but I can't even get the other one working first.
}
// setup the visualization tree, unfortunately this needs to happen before snapshot can be created.
// The update method creates a new key/value pair representing the full name so that it's easier to
// search using Defiant.
root.x0 = width / 2;
root.y0 = height / 2;
update(root);
centerNode(root, true, true);
scope.loaded = true;
// copy the root so we can remove parent keys causing Defiant infinite loops
context = angular.copy(root);
removeParentKey(context);
// attempt to create the snapshot
Defiant.getSnapshot(context, function(snap) {
console.log(snap); // this never occurs
contextSnap = snap; // neither does this.
});
} My use case is I try to create this snapshot shortly after tree rendering so that the user can use the search function any time after that. I am stumped and why this isn't working... |
Hi @Mctalian, Anyhow...before troubleshooting, my first question is if "json-search-6.htm" works as expected when you try it out. If it did, and you used the concatenated + minified from this branch, the resulting defiantjs-lib will not contain the new feature. The reason is that this feature-branch is not "grunted". However, I've grunted it now and this minified version contains the latest code: If you do have a demo version with the code above, online - can you paste it here...I think I could debug in browser and see what might be going on. Thanks |
@hbi99 Sorry about that. I wasn't sure how smart the comment system was. I'll keep that in mind for the future. This is how I tested it. I downloaded a zip of the project, ran grunt on it and used the generated files. When that didn't work, I copied the source files over to my project and tried it that way with the same results. I will try to work on getting a demo version available today and this time I will leave a new comment :P |
@hbi99 I was able to create a demo version, and unfortunately for me, it works fine. Just like "json-search-6.htm". I tried to replicate everything I could; the data, the angular directive where the tree creation occurs, the defiant service where I dynamically add the defiant.js script tag to the bottom of the file. The demo works, my setup does not. I am pretty stumped. Everything I've tried works except where I need it to, in my project. |
@Mctalian, ...
removeParentKey(context);
console.log( JSON.stringify(content, null, '\t') );
console.log(Defiant);
// attempt to create the snapshot
Defiant.getSnapshot(context, function(snap) {
... I added two logger lines above...I suspect, one of them should not log as expected but;
...can you try adding these two lines and see if any of them return something else than expected? |
I was able to refactor a bit: setUpData(root); // a minimal 'update' method
removeParentKey(root);
console.log(JSON.stringify(root, null, '\t'));
console.log(Defiant);
contextSnap = Defiant.getSnapshot(root);
update(root);
centerNode(root, true, true);
scope.loaded = true;
console.log(contextSnap); This printed out in the following order:
And this worked fine, but the UI unresponsiveness is still present. Attempting the same but with the callbacks: setUpData(root); // a minimal 'update' method
removeParentKey(root);
console.log(JSON.stringify(root, null, '\t'));
console.log(Defiant);
// attempt to create the snapshot
Defiant.getSnapshot(root, function (c) {
console.log(c); // this never occurs
contextSnap = c; // neither does this.
update(root);
centerNode(root, true, true);
scope.loaded = true;
});
console.log(contextSnap); This printed out in the following order:
But the code inside the getSnapshot callback never executed. |
@Mctalian |
@hbi99, yeah it's really weird that I've gotten it working in 2 different scenarios, but can't get it working in the one that matters. I tried to reproduce it on JsFiddle and it was non reproducible. I will try to look into it more tomorrow and even more throughout the week. I'm going to add some prints in defiant code to see if that adds any insight. |
Alright I may have gotten somewhere: https://github.com/Mctalian/FraterniTree-Lite If you get that running and paste the data into If you go into the file contextSnap = defiant.getSnapshot(root);
update(root);
centerNode(root, true, true);
scope.loaded = true; and //defiant.getSnapshot(root, function (c) {
// console.log(c); // this never occurs
// contextSnap = c; // neither does this.
// update(root);
// centerNode(root, true, true);
// scope.loaded = true;
//}); Comment out the first, and uncomment out the second and you should be able to reproduce it. It looks like it's a call stack issue, not sure why I couldn't see the error before... Let me know if you need any help setting it up. I am kind of rushed in setting this up. Thanks! |
In my haste, I have given slightly incorrect instructions. I have updated the post above to reflect how to do it correctly. I even used the sample data that is present on the FraterniTree-Lite README and I still see the same issue. |
While drinking coffee from my mug, It turns out that you are including the master-branch version of defiant in your "app.min.js". Look at this picture...this path includes version 1.5.2 - which doesn't expose the new feature. PS: I write rhymes much better in swedish ;-) |
If you are looking at my live site, yes that is true. I develop locally and deploy once I get things working. I haven't been able to get the new snapshots working, so I am using the master branch. If you look at the demo I've made for you, you will see that I am using the web-worker branch version:
I wish it were so easy :( |
My bad...x10.js was not included in the ww-branch's min-version. To make sure x10 is present, in the browser, type "x10" and hit "enter". |
I'm not sure that's it, either. I was manually adding x10 with all my prior tests. With the new version it seems that I no longer need to manually include x10 in my index.html, but I am still seeing a callstack exceeded error. If you try my demo project, you'll now have to update DefiantService.js to include a local path to the new defiant.min.js that you generated for the web-worker branch. The CDN link I use takes a while to propagate it seems. I recommend putting |
Got it...! |
Ah that makes sense! I think that is naturally added somewhere along the chain, I'm honestly not sure where because I tried to duplicate this in a fiddle with almost identical components to no avail. Regardless I can likely remove that property and get it working. I'll also look into where that's being added as well. Thanks for looking into that! I'll work on implementing the fix this weekend. What's the ETA for this getting integrated with the main branch? Or would you rather me test it out first? |
OH! It looks like that's added when using $resource in angular. This is how I'm getting the data from the server. |
Thanks for looking into that, I checked it out and remove the $promise property worked like a charm in my small test. It's also much much faster which is always great. This is a great library and your quick responses and support are much appreciated. Great work! |
@Mctalian No probs :) ...but I chose to not do anything when/if this occurs. Now, since you have encountered this, how would you've expect the lib to handle this scenario? The alternatives are:
The ETA of this feature might be this weekend...I do have a lot of other things to do but I am hoping to do it anyway. Until then, if you try it out and hopefully catch buggs would be optimal. |
Not sure if you want a new issue for this, but I have a use case where I need to create a snapshot, then do a search, and take a snapshot of the results of the search to use for later searches. I find that when I do this, it creates an infinite loop between the 2 getSnapshots. I was able to replicate this using your var test = {
init: function(data) {
var snapshot,
found,
start = Date.now();
Defiant.getSnapshot(data, function(snapshot) {
console.log('Created snapshot with web worker in '+ (Date.now() - start) +' ms');
// test 3
var found = JSON.search(snapshot, '//friends[contains(name, "mona")]');
console.log(found);
Defiant.getSnapshot(found, function(snap)
{
console.log('Created snapshot with web worker in '+ (Date.now() - start) +' ms');
});
});
}
}; Hopefully I am just going about this the wrong way and there's a simple way to change my approach to make this work. |
But to answer your question on what I'd expect the lib to do: I would say at the very least, error so users like me aren't left scratching their heads. Perhaps another solution is to provide a warning and keep moving so if users experience something weird, they can see it in the log and make adjustments. I'd think option 2 is best if it warns the user what it removed and why. Users can make adjustments if they see fit, or just let it do its thing. |
You should not try to search on found items. First of all, the found objects are mutable...also, there is no need to try to improve on speed once you have a snapshot. In your case, your JSON file is very small...about 80Kb. I have tested searching on snapshots on files of 1.6MB size and in that scenario, the search took 4-10 ms...This is veeeerrry fast and really enough. If you're dealing with larger data than this, than you probably should re-think the architecture of what your solution. Finally, the snapshot that DefiantJS created is not JSON structure...it is just a snapshot of objects required to make fast searches...you might interpret them as "shortcuts" if you want. |
Perfect, you're right. I did not need the extra snapshot. My search method was already taking care of nodes that were not visible on the screen and I forgot about that. |
If you look at the following web page and continuously highlight and unhighlight a link while the tree is loading, you will notice that the link stops highlighting or gets stuck highlighted for 0.5 to 2 seconds:
http://www.yitbosoft.com/orgs/delta_sigma_phi/Alpha_Eta
The data that is being snapshotted is:
http://www.yitbosoft.com/api/v2/orgs/delta_sigma_phi/Alpha_Eta
The text was updated successfully, but these errors were encountered: