Multiple messages with the same old name#3013
Conversation
44d5236 to
9268001
Compare
9268001 to
79e81fe
Compare
Also fixed spelling in C0112 to C0116 messages following the review of pull-request pylint-dev#2036 by Ashley Whetter. See also pylint-dev#2075, pylint-dev#2077, pylint-dev#2262, pylint-dev#2654, pylint-dev#2805, pylint-dev#2809, pylint-dev#2844, pylint-dev#2992 and pylint-dev#3013 for full historical context.
79e81fe to
6278b85
Compare
|
I made some performance review (This is why I removed the MessageId class). Regarding performance, I think this is now making things better but it's not very clear cut: Benchmark usedPylint analysing itself (~= 65s) :Total timeSum of the time for the function containing "message_"Pylint analysing astroid (~= 1,25s) :Not shown here because it's too fast. BeforeTotal timeSum of the time for the function containing "message_"Longest message functionAfterTotal timeSum of the time for the function containing "message_"Longest message functionConclusionIt seems that overall the analysis is faster, but the time sepcifically for the part we changed is a little longer. We could probably merge MessageIdStore and MessageDefinitionStore to have less function call (51484 call to get_active_msgids are added) but I think it would affect maintenability a little. |
6278b85 to
cad617d
Compare
cad617d to
736db37
Compare
It seems to make performance 2/3 % better during test and during pylint own analysis.
736db37 to
2a266cc
Compare
|
@PCManticore this one is ready for review, it should be a piece of cake after #2992 ;) |
Also fixed spelling in C0112 to C0116 messages following the review of pull-request pylint-dev#2036 by Ashley Whetter. See also pylint-dev#2075, pylint-dev#2077, pylint-dev#2262, pylint-dev#2654, pylint-dev#2805, pylint-dev#2809, pylint-dev#2844, pylint-dev#2992 and pylint-dev#3013 for full historical context.
|
Hi, @PCManticore, I won't be super available from mid-September to the end of October, now would be the best time to review this and #3021 :) |
|
Awesome stuff @Pierre-Sassoulas ! 👍Sorry for the delay in getting to this, I was out as well. |
Also fixed spelling in C0112 to C0116 messages following the review of pull-request pylint-dev#2036 by Ashley Whetter. See also pylint-dev#2075, pylint-dev#2077, pylint-dev#2262, pylint-dev#2654, pylint-dev#2805, pylint-dev#2809, pylint-dev#2844, pylint-dev#2992 and pylint-dev#3013 for full historical context.
Also fixed spelling in C0112 to C0116 messages following the review of pull-request pylint-dev#2036 by Ashley Whetter. See also pylint-dev#2075, pylint-dev#2077, pylint-dev#2262, pylint-dev#2654, pylint-dev#2805, pylint-dev#2809, pylint-dev#2844, pylint-dev#2992 and pylint-dev#3013 for full historical context.
Description
Permit to have multiple messages with the same old name. There are optimisations done that make Pylint a little faster.
Type of Changes
Related Issue
This is it, after this PR the real review for #1164 can start.