{"action":"created","issue":{"url":"https://api.github.com/repos/netty/netty/issues/3570","labels_url":"https://api.github.com/repos/netty/netty/issues/3570/labels{/name}","comments_url":"https://api.github.com/repos/netty/netty/issues/3570/comments","events_url":"https://api.github.com/repos/netty/netty/issues/3570/events","html_url":"https://github.com/netty/netty/issues/3570","id":66017447,"number":3570,"title":"Performance of Http2Stream.setProperty(...) / .getProperty(...).","user":{"login":"buchgr","id":1388179,"avatar_url":"https://avatars.githubusercontent.com/u/1388179?v=3","gravatar_id":"","url":"https://api.github.com/users/buchgr","html_url":"https://github.com/buchgr","followers_url":"https://api.github.com/users/buchgr/followers","following_url":"https://api.github.com/users/buchgr/following{/other_user}","gists_url":"https://api.github.com/users/buchgr/gists{/gist_id}","starred_url":"https://api.github.com/users/buchgr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/buchgr/subscriptions","organizations_url":"https://api.github.com/users/buchgr/orgs","repos_url":"https://api.github.com/users/buchgr/repos","events_url":"https://api.github.com/users/buchgr/events{/privacy}","received_events_url":"https://api.github.com/users/buchgr/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/netty/netty/labels/feature","name":"feature","color":"009800"},{"url":"https://api.github.com/repos/netty/netty/labels/improvement","name":"improvement","color":"0052cc"}],"state":"open","locked":false,"assignee":null,"milestone":{"url":"https://api.github.com/repos/netty/netty/milestones/112","html_url":"https://github.com/netty/netty/milestones/4.1.0.Beta5","labels_url":"https://api.github.com/repos/netty/netty/milestones/112/labels","id":1005380,"number":112,"title":"4.1.0.Beta5","description":"","creator":{"login":"normanmaurer","id":439362,"avatar_url":"https://avatars.githubusercontent.com/u/439362?v=3","gravatar_id":"","url":"https://api.github.com/users/normanmaurer","html_url":"https://github.com/normanmaurer","followers_url":"https://api.github.com/users/normanmaurer/followers","following_url":"https://api.github.com/users/normanmaurer/following{/other_user}","gists_url":"https://api.github.com/users/normanmaurer/gists{/gist_id}","starred_url":"https://api.github.com/users/normanmaurer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/normanmaurer/subscriptions","organizations_url":"https://api.github.com/users/normanmaurer/orgs","repos_url":"https://api.github.com/users/normanmaurer/repos","events_url":"https://api.github.com/users/normanmaurer/events{/privacy}","received_events_url":"https://api.github.com/users/normanmaurer/received_events","type":"User","site_admin":false},"open_issues":75,"closed_issues":42,"state":"open","created_at":"2015-03-04T09:22:23Z","updated_at":"2015-04-05T20:01:17Z","due_on":null,"closed_at":null},"comments":16,"created_at":"2015-04-02T21:44:48Z","updated_at":"2015-04-06T01:01:28Z","closed_at":null,"body":"I have been doing some benchmarking and profiling on the HTTP/2 codec. One of the pain points is that the codec is spending too much time and memory on `Http2Stream.setProperty(...) / .getProperty(...)`. In my benchmarks we spend roughly 11% of the CPU time on retrieving and putting \"properties\" and use just over 22% of the heap for `HashMaps`.\r\n\r\nThe main reason that we spend so much time on the properties is because of the local and remote `FlowState`. We look it up all the time. The main reason why we are using so much memory for `HashMap` is that each stream uses its own.\r\n\r\nMy preferred way to solve this is:\r\n\r\n* Add `remoteFlowState()` and `localFlowState()` properties to `Http2Stream`. Use those instead of the properties.\r\n* Make `(set|get)Property` package private and initialize the `HashMap` backing this lazily (right now that's only used in `CompressorHttp2ConnectionEncoder` and `DelegatingDecompressorFrameListener`). That way we only pay the overhead of a `HashMap` per stream when those classes are used and we have the flexibility to attach things to the stream in future code.\r\n* Introduce public interface methods `setUserProperty(Object val) / getUserProperty()`. Those are not used from within the codec package, but a user is free to attach whatever object he feels like.\r\n\r\nWDYT @nmittler @Scottmitch "},"comment":{"url":"https://api.github.com/repos/netty/netty/issues/comments/89877329","html_url":"https://github.com/netty/netty/issues/3570#issuecomment-89877329","issue_url":"https://api.github.com/repos/netty/netty/issues/3570","id":89877329,"user":{"login":"buchgr","id":1388179,"avatar_url":"https://avatars.githubusercontent.com/u/1388179?v=3","gravatar_id":"","url":"https://api.github.com/users/buchgr","html_url":"https://github.com/buchgr","followers_url":"https://api.github.com/users/buchgr/followers","following_url":"https://api.github.com/users/buchgr/following{/other_user}","gists_url":"https://api.github.com/users/buchgr/gists{/gist_id}","starred_url":"https://api.github.com/users/buchgr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/buchgr/subscriptions","organizations_url":"https://api.github.com/users/buchgr/orgs","repos_url":"https://api.github.com/users/buchgr/repos","events_url":"https://api.github.com/users/buchgr/events{/privacy}","received_events_url":"https://api.github.com/users/buchgr/received_events","type":"User","site_admin":false},"created_at":"2015-04-06T01:01:28Z","updated_at":"2015-04-06T01:01:28Z","body":"@Scottmitch yes. I left the `(set|get)Property` untouched and just didn't use them.\r\n\r\n@nmittler \r\n> The same applies for the user property ... maybe they need multiple.\r\n\r\nAs @Scottmitch said a user is free to plug in whatever, be it his own \"property object\" or a map.\r\n\r\n> I guess I'm not quite sure what is causing you heartburn about that idea? It's not wasting space and lookup time will be O(1).\r\n\r\nI think the API is just way too complex with having to request property keys and such. In terms of performance, I don't know. While fields and arrays are both constant time access, my guess would be that that constant for fields is a lot smaller, but I don't know. Would have to benchmark.\r\n\r\nAlso something to consider is that we probably have to make that user object field volatile, and there is no way to make an array element volatile in Java. Anyways, my main concern is really the API and I don't see any downside with the user object. Having that user object just \"feels natural\" to me and not like a hack.\r\n\r\n@nmittler @Scottmitch \r\n\r\nRegarding what to do about \"codec properties\". While I am fine with just adding package private setters / getters, I would also be fine with just keeping the current hashmap for that. Just make `(set|getProperty)` package private. Yes, it's a bit slower. Yes, it uses more memory. However, it's not used much and as long as nobody complains we can likely get away with it.\r\n\r\nRegarding the flow state. I think we all agree on having it as a field.\r\n\r\nRegarding the user property. I would argue, similar to the flow state, every application using the HTTP/2 codec will make use of it. One will just always need to maintain some state / information per stream, because the way the HTTP/2 codec is written, there are no channelhandlers. In a \"natural Netty codec\" every channelhandler has a few fields where it maintains state, in a HTTP/2 application one would have to attach it to the stream instead. So that's why I feel it also makes sense to have the user object as a field.\r\n"}}