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

Startup time optimization - redundant ObjectMappers #91

Closed
cslee00 opened this issue Jan 10, 2018 · 7 comments
Closed

Startup time optimization - redundant ObjectMappers #91

cslee00 opened this issue Jan 10, 2018 · 7 comments
Assignees
Milestone

Comments

@cslee00
Copy link

cslee00 commented Jan 10, 2018

Profiling to reduce lambda cold-start time (Spring context, not Boot).

There are 2 or 3 ObjectMappers created, which take a moderate amount of time to initialize:

  1. inside AwsProxyRequestBuilder (new ObjectMapper())
  2. inside AwsProxyExceptionHandler (new ObjectMapper())
  3. user-created when using RequestStreamHandler (new ObjectMapper())

#1 & #2 could be hoisted out to a single instantiation.

Ideally the same ObjectMapper would be used for #3, possibly through handler.proxy( inputStream, context) to hide this inside the container.

@sapessi
Copy link
Collaborator

sapessi commented Jan 10, 2018

Nice, thanks for the suggestions @cslee00 - I'll make sure to do this for the next release.

@sapessi sapessi self-assigned this Jan 10, 2018
@sapessi sapessi added this to the Release 0.9 milestone Jan 10, 2018
@sapessi
Copy link
Collaborator

sapessi commented Jan 10, 2018

The first object mapper, in AwsProxyRequestBuilder, is only used for unit tests. It should no be initialized at runtime. Do you still see it being created?

@cslee00
Copy link
Author

cslee00 commented Jan 10, 2018

No, apologies, was reading from a search for new ObjectMapper() vs the profiler - nothing there for AwsProxyRequestBuilder, just the one (#2 above) inside the framework. Would be great to get down to one mapper for the container, and one for Spring MVC, as ObjectMapper creation is the third hotspot in the profiler (3 instances - #2 & #3 above, and one for Spring MVC which needs to be separate).

@sapessi
Copy link
Collaborator

sapessi commented Jan 10, 2018

I'm moving the object mapper to be a singleton inside the LambdaContainerHandler class. All classes in the framework will re-use the singleton and stream handler implementations can do the same. I'll think through what I can do to make it easier to build stream handlers as a separate issue.

@cslee00
Copy link
Author

cslee00 commented Jan 10, 2018

Thx!

In my stream handler implementation (below) there is nothing specific to my code base, nor are there foreseeable points where there would be value to hooking into. The entire class could be provided by the container, with documentation on what to specify as the Lambda entry point (with some consideration to extension points for subclasses if need be).

It seems this could also replace the need for implementations of RequestHandler, unifying the serialization mechanism (minor concern in having two different ways of serializing/deserializing the same message format)

public final class LambdaHandler implements RequestStreamHandler {
    private final ObjectMapper mapper = new ObjectMapper();
    SpringLambdaContainerHandler<AwsProxyRequest, AwsProxyResponse> handler;

    public LambdaHandler() throws Throwable {
        handler = SpringLambdaContainerHandler.getAwsProxyHandler(AppConfig.class);
    }

    @Override
    public void handleRequest(InputStream inputStream, OutputStream outputStream, Context context) throws IOException {
        try {
            AwsProxyRequest request = mapper.readValue(inputStream, AwsProxyRequest.class);
            AwsProxyResponse resp = handler.proxy(request, context);
            mapper.writeValue(outputStream, resp);
        } finally {
            // just in case it wasn't closed by the mapper
            outputStream.close();
        }
    }
}```

@sapessi
Copy link
Collaborator

sapessi commented Jan 10, 2018

We could provide a stream handler class for each framework. So far, we've tried to stay away from it because developers may want to perform additional business logic in the handler such as configuration for X-Ray. My plan was to publish some archetypes with version 1 of the framework and include the default stream handler in there.

sapessi added a commit that referenced this issue Jan 11, 2018
…Mapper to speed up initialization and address #91. Added unit tests to make sure #90 is resolved and stays resolved.
@sapessi
Copy link
Collaborator

sapessi commented Jan 11, 2018

Closing this since it was addressed in the latest commit. It will go out with version 0.9. I will also update the samples to add a stream handler that re-uses the same ObjectMapper

@sapessi sapessi closed this as completed Jan 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants