Skip to content

Conversation

@taooceros
Copy link
Member

@taooceros taooceros commented Oct 17, 2020

Wox has changed the Pinyin Library to ToolGood.Words. #2962 Thank you for the change @bao-qian .
This pull request has done the same thing, but optimize a few.

  1. It keeps the full pinyin query, which seems perform the same as current pinyin library.
  2. It keeps using the Concurrent Dictionary as cache, instead of Wox's Memory Cache. I think changing the cache implementation should be done in separately. However, the cache for Pinyin is changed from store single chinese character to whole word.
  3. IAlphabet interface is kept in this pull request.
  4. I also merge the query of win32 and uwp in Program plugin into one query by adding enabled property to the IProgram interface and cast win32 and uwp to IProgram.

I have checked the previous cache storage and it actually makes the loading time quite long, so I remove the Save and Loading cache in this pull request because it seems that the library is fast enough.

@jjw24
Copy link
Member

jjw24 commented Oct 18, 2020

Hey how's it going. Do we know the advantages of the new library over the current? Are there benefits aside from faster loading time?

I have checked the previous cache storage and it actually makes the loading time quite long, so I remove the Save and Loading cache in this pull request because it seems that the library is fast enough.

Is there a way we can test this just to be sure?

@taooceros
Copy link
Member Author

taooceros commented Oct 18, 2020

Do we know the advantages of the new library over the current?

  1. It seems that the query time has been much faster. By using this branch as release comparing to master, the query time has reduced from approximately 13ms to 1ms. This should majorly because that we don't need to compare all of the comparison of polyphone.
    image
    The init time is longer may due to that I don't store the cache in my current code, but that is still lower than the initialization time for the current pinyin library.

  2. The library support polyphone natively, so that we don't need extra combination, which largely increase the query time. Also, this reduce the memory used for cache, though the memory used for the library is larger, so eventually is almost the same, or a little bit lower. In my computer, the memory used has been reduced about 80Mb.

Is there a way we can test this just to be sure?

I am not sure whether there is a proper way to do that...but I think the init time for query for Program plugin can be a reference, because only the first time will utilize library to generate the pinyin cache.
I will try to remove the cache and check the query time.

@jjw24
Copy link
Member

jjw24 commented Oct 18, 2020

Sounds like an awesome change if query time and memory utilisation with pinyin search is less. let me know when PR is ready for review.

@jjw24
Copy link
Member

jjw24 commented Oct 18, 2020

Also sorry i forgot to mention, if we are reusing code from Wox, lets attribute the commits to the authors that wrote it please.

@taooceros
Copy link
Member Author

Sure, could you please tell me how I should do that?

@taooceros taooceros marked this pull request as ready for review October 18, 2020 03:48
@jjw24
Copy link
Member

jjw24 commented Oct 18, 2020

Sure, could you please tell me how I should do that?

yeah so when you commit the changes use this to indicate the author, you can find his email on his profile page. Also please let's not write the commit message 'code from Wox', but rather describe the changes as usual.
git commit --author="FIRST LASTNAME <NAME@EMAIL.com>" -m "YOUR MESSAGE THAT DESCRIBES THE COMMIT"

Apologies if this makes you have to re-PR

@taooceros
Copy link
Member Author

Let me take a try on a new branch

@taooceros
Copy link
Member Author

Could you give me a help?
image

@jjw24
Copy link
Member

jjw24 commented Oct 18, 2020

you need to keep the angle brackets <>

git commit --author="John blah <JohnBlah@someemail.com>" -m "YOUR MESSAGE THAT DESCRIBES THE COMMIT"

@taooceros taooceros mentioned this pull request Oct 18, 2020
@taooceros
Copy link
Member Author

Done it and created a new pull request #183

@taooceros taooceros closed this Oct 18, 2020
@taooceros taooceros deleted the PinyinImprovement branch October 18, 2020 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants