Skip to content
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.

Sign up for GitHub

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

Node Search performance improvements #12056

Merged
merged 15 commits into from
Oct 4, 2021
Merged

Node Search performance improvements #12056

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
06d0ad3
first pass
pinzart Sep 23, 2021
ec2fd27
Update SearchDictionary.cs
pinzart Sep 23, 2021
4ad7d64
Update SearchDictionary.cs
pinzart Sep 23, 2021
a4a0298
Update SearchDictionary.cs
pinzart Sep 23, 2021
ca08dd7
update
pinzart Sep 23, 2021
6344b71
improvements
pinzart Sep 27, 2021
6f1341d
Merge branch 'master' into simplify_search
pinzart Sep 27, 2021
3c4eca2
add unit performance unit test
pinzart Sep 27, 2021
cc1db45
test seachCommand invoke count
pinzart Sep 27, 2021
7124644
Create searchtags.log
pinzart Sep 27, 2021
ebe06b4
update
pinzart Sep 27, 2021
7be230c
scramble data
pinzart Sep 27, 2021
ab05505
Update SearchDictionaryTest.cs
pinzart Sep 28, 2021
ce3999d
Merge branch 'master' into simplify_search
pinzart Oct 1, 2021
3216447
add hidden option
pinzart Oct 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/DynamoCore/Search/SearchDictionary.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using System.Collections.Generic;
using System.Linq;
using System.Diagnostics;
using System.Configuration;
using System.Reflection;

namespace Dynamo.Search
{
Expand All @@ -15,6 +17,22 @@ public class SearchDictionary<V>
private ILogger logger;
private static int LIMIT_SEARCH_TAG_SIZE = 300;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we all agree that 300 characters is good enough ?
Should we increase ?

Looking through the search tags we got from Jostein, I found the following:

  1. The longest search tag is 899 characters
  2. There are 34 search tags (out of 6525) that are over 300 characters

Copy link
Contributor Author

@pinzart90 pinzart90 Sep 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ALso this limit will be used in the Package Search window as well..(using package descriptions)
We could use another limit for Package Search ....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @mjkkirschner @aparajit-pratap

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine by me but I'm still confused - these search tags from Jostein look like node descriptions. I'm curious if the entire description is used as a search tag, then what's the benefit of the search tags included in the <search></search> XML?

Copy link
Member

@mjkkirschner mjkkirschner Sep 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aparajit-pratap it's to inject extra search terms without adding them to the description.
So I can add box to the search terms for Cuboid - without needing to add box to the description.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pinzart90 what do you think about making this a hidden preference? I know it's more work, and kind of a pain, but it let's users tweak it if they run into trouble.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjkkirschner I added a new assembly config for it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, sorry @pinzart90 - I was not clear, by hidden preference -I just meant a preference in the dynamosettings.xml with no analog in the UI to control it. This is fine as well, users can still modify this if we want them to test something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjkkirschner if you think there is good enough reason to move to Preferences, then I can do that

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be a good idea to move it to the preference settings file so we have all preferences in one place. That is usually the one file where all such user preferences go.


static SearchDictionary()
{
try
{
// Look up search tag limit in the assembly configuration
var assemblyConfig = ConfigurationManager.OpenExeConfiguration(Assembly.GetExecutingAssembly().Location);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the DynamoCore.dll.config file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

if (assemblyConfig != null)
{
var searchTagSizeLimit = assemblyConfig.AppSettings.Settings["searchTagSizeLimit"];
if (searchTagSizeLimit != null && int.TryParse(searchTagSizeLimit.Value, out int value))
LIMIT_SEARCH_TAG_SIZE = value;
}
}
catch { }
}

/// <summary>
/// Construct a SearchDictionary object
/// </summary>
Expand Down