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

Performance Issue in 2.10.1 jackson.databind.ObjectMapper.convertValue #2645

Closed
GORA-SAG opened this issue Mar 6, 2020 · 5 comments
Closed
Labels
will-not-fix Closed as either non-issue or something not planned to be worked on

Comments

@GORA-SAG
Copy link

GORA-SAG commented Mar 6, 2020

Upgraded the jackson-databind jar from 2.8.7 to 2.9.9.3. We faced security vulnerabilities in that version. Then we upgraded to 2.10.1 version of jackson-databind. But after upgrading to this version we are facing performance degradation in com.fasterxml.jackson.databind.ObjectMapper.convertValue() method. Before the update, it took 1.8ms to process the request after the upgrade it took 8.5second to process the request. This causing major performance degradation.

We also tested the latest versions 2.10.2 and 2.10.3 facing the same issue again. Can you please have a look and provide the fix in 2.10.4. Thanks in advance.

@cowtowncoder cowtowncoder added the need-test-case To work on issue, a reproduction (ideally unit test) needed label Mar 6, 2020
@cowtowncoder
Copy link
Member

@GORA-SAG This is not enough information for anyone to look into anything.

Please provide a test case showing the performance degradation.

@GORA-SAG
Copy link
Author

@cowtowncoder We are having Petstore API with multiple resources. While invoking any of the resource we are parsing the paths of the API using jackson-databind ObjectMapper.convertValue() method. We are converting the path object to our Path class. During that operation we are facing the performance degradation issue.

Is this enough or do you need more information.

@cowtowncoder
Copy link
Member

@GORA-SAG no, I would need a reproduction of some kind. Explaining general idea, in context of a framework, does not help unfortunately. We have not received other reports for this functionality, performance impacts.

But one guess on root cause: I suspect change that triggers more processing is #2220, and solution (if so) is to fix problems in calling code, not Jackson -- this only affects cases where formerly no conversion was done (input type already compatible with result type). And if so, code should probably just see if any conversion is needed at all, or not: if not, do not call convertValue() but just use regular Java type cast.

@GORA-SAG
Copy link
Author

GORA-SAG commented Mar 31, 2020

Please run this code snippet with jackson-databind.jar. Upto 2.10.1 you will get 1ms as output but after using 2.10.1 and above version you will get 55-60ms.

import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.HashMap;

public class PerformanceTest {
    public static void main(String[] args) {
        ObjectMapper mapper = new ObjectMapper();
        Path path = new Path();
        path.setMethod("GET");
        path.setUrl("https://google.com");
        path.setPathParams("username=test");
        path.setQueryParams("password=test");
        path.setRequestBody("{\"IN\": \"123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123\"} ");
        path.setResponseBody("{\"IN\": \"123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123\"} ");
        HashMap<String, String> headers = new HashMap<>();
        headers.put("APIKey", "1234567890");
        headers.put("Authorization", "Basic test:test");
        long startTime = System.currentTimeMillis();
        mapper.convertValue(path, Path.class);
        long endTime = System.currentTimeMillis();
        System.out.println(endTime - startTime);
    }
}

class Path {
    private String method;
    private String url;
    private String requestBody;
    private String responseBody;
    private HashMap<String,String> headers;
    private String queryParams;
    private String pathParams;

    public String getMethod() {
        return method;
    }

    public void setMethod(String method) {
        this.method = method;
    }

    public String getUrl() {
        return url;
    }

    public void setUrl(String url) {
        this.url = url;
    }

    public String getRequestBody() {
        return requestBody;
    }

    public void setRequestBody(String requestBody) {
        this.requestBody = requestBody;
    }

    public String getResponseBody() {
        return responseBody;
    }

    public void setResponseBody(String responseBody) {
        this.responseBody = responseBody;
    }

    public HashMap<String, String> getHeaders() {
        return headers;
    }

    public void setHeaders(HashMap<String, String> headers) {
        this.headers = headers;
    }

    public String getQueryParams() {
        return queryParams;
    }

    public void setQueryParams(String queryParams) {
        this.queryParams = queryParams;
    }

    public String getPathParams() {
        return pathParams;
    }

    public void setPathParams(String pathParams) {
        this.pathParams = pathParams;
    }
}

@cowtowncoder
Copy link
Member

@GORA-SAG Ok so the problem is your use of convertValue; you should not do this:

Path path = ....

Path result = mapper.convertValue(path, Path.class)

since... why would you try to convert from Path to Path?

Solution here is simple: do not call convertValue() if value is already of type you have, unless you have very specific need to do that (like converting from one Map/List to another).

Change you observe is due to #2220 -- instead of skipping just (usually) pointless cases, full conversion is now made: serialize to a buffer, deserialize from buffer.
This solves some cases where users wanted to convert from, say, Map<String, JsonNode> into Map<String, SomePOJO> (or Lists).

@cowtowncoder cowtowncoder added will-not-fix Closed as either non-issue or something not planned to be worked on and removed need-test-case To work on issue, a reproduction (ideally unit test) needed labels Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
will-not-fix Closed as either non-issue or something not planned to be worked on
Projects
None yet
Development

No branches or pull requests

2 participants