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.

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

Allow parsing request and writing responses using java streams #41

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ansman
Copy link

@ansman ansman commented Dec 17, 2019

This fixes #39

@ansman ansman force-pushed the master branch 3 times, most recently from 968dff7 to 0cc75c1 Compare December 17, 2019 15:25
@@ -201,6 +201,145 @@ internal class AogRequest internal constructor(
companion object {
private val LOG = LoggerFactory.getLogger(AogRequest::class.java.name)

private val gson = GsonBuilder()
Copy link
Author

Choose a reason for hiding this comment

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

Allocating Gson instances semi heavy and should be cached. Ideally one Gson instance for the whole SDK should be used. Since they are immutable it seems like a reasonable thing to do.

Copy link
Member

Choose a reason for hiding this comment

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

How many times is create called during a particular request? Will it have any performance changes?

Copy link
Author

Choose a reason for hiding this comment

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

Probably only once, but it will create a new instance for every server call for no good reason. There is no reason not to cache the instance.

Copy link
Member

@Fleker Fleker left a comment

Choose a reason for hiding this comment

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

Overall LGTM but I'll probably cherry-pick it and see if I can build a sample with it.

@taycaldwell I'd like your comments as well.

@@ -201,6 +201,145 @@ internal class AogRequest internal constructor(
companion object {
private val LOG = LoggerFactory.getLogger(AogRequest::class.java.name)

private val gson = GsonBuilder()
Copy link
Member

Choose a reason for hiding this comment

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

How many times is create called during a particular request? Will it have any performance changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow deserializing a request and serializing a response using streams
2 participants