Add a reset() method in Message #40
Conversation
The clear() method is public but its purpose is not explained. It does not allow to reuse a message because it removes the header & trailers of a message. This reset() method can be used to reuse a message.
The clear() method is public but its purpose is not explained. It does not allows to reuse a message because it removes the header & trailers content of a message. This reset() method can be used to reuse a message.
|
TreeMap provides ordering. Probably should have some level of tests for this. |
|
I don't remember that I PR the change from TreeMap to ArrayList, but the ordering was easily kept using list. Regarding the size of most fix messages (number of fields), sorting a list on insertion instead of a using a tree was not a big speed loose but avoid many allocations…
Le 7 nov. 2017 à 01:22, Philip a écrit :
… TreeMap provides ordering.
Probably should have some level of tests for this.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
I don't really understand why charlesbr1's PRs get so little attention. |
|
I suppose writing garbage less code is not common, and unfortunately this work would be hard to merge because they were internal changes in quickfix that I didn’t follow. I’m not working with quickfix anymore...
… Le 8 nov. 2017 à 10:44, Martin Vseticka ***@***.***> a écrit :
I don't really understand why charlesbr1's PRs get so little attention.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Hi @MartyIX , @charlesbr1 , the answer to this is pretty simple: lack of time. My employer allows me to work a few hours per week on this open source project and I focus on implementing stuff that needs to be done or otherwise QFJ users will run into problems (e.g. support for higher precision time stamps). Additionally, the focus in QFJ is not performance. Of course, there is always room for improvement (and there were some performance-related changes done in the past) but I personally do not have time to work through every PR that claims that this or that has been improved. This is not because I do not trust the one that created the PR but because I do not have time to test all of this. IMHO there should be something like a JMH benchmark or GC logs so that everyone can reproduce the optimizations. Moreover, as current maintainer of this project I do not want to introduce too much risk shortly before a release. That being said, reviewers are always welcome. And I also noticed that more and more people review/comment on the various PRs. Thanks to all people that already did. :) Cheers, |
|
You make good work Chris !
I understand time is always lacking, and there is other framework designed for performance. Quickfix is well know and pleasant to use. May be in the futur if I got an opportunity to use it again I’ll spend more time on PR, but for the moment I can not...
… Le 8 nov. 2017 à 13:39, Christoph John ***@***.***> a écrit :
Hi @MartyIX , @charlesbr1 ,
the answer to this is pretty simple: lack of time. My employer allows me to work a few hours per week on this open source project and I focus on implementing stuff that needs to be done or otherwise QFJ users will run into problems (e.g. support for higher precision time stamps).
Additionally, the focus in QFJ is not performance. Of course, there is always room for improvement (and there were some performance-related changes done in the past) but I personally do not have time to work through every PR that claims that this or that has been improved. This is not because I do not trust the one that created the PR but because I do not have time to test all of this. IMHO there should be something like a JMH benchmark or GC logs so that everyone can reproduce the optimizations.
Moreover, as current maintainer of this project I do not want to introduce too much risk shortly before a release.
I also contacted Charles a few months ago if he could update his PRs to the current state of the project. But I understood that he is not involved in QFJ anymore and so it is difficult for him.
That being said, reviewers are always welcome. And I also noticed that more and more people review/comment on the various PRs. Thanks to all people that already did. :)
Cheers,
Chris.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
The clear() method is public but its purpose is not explained.
It does not allows to reuse a message because it removes the header & trailers content of a message.
This reset() method can be used to reuse a message.
It would be much efficient to reuse messages if we have List instead of TreeMap in FieldMap class, to store fields and groups. Because TreeMap allocate a bucket for each entry, and they cannot be recycled.