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

Reduce memory usage when reading a large script file. #101

Merged
merged 2 commits into from Nov 18, 2017

Conversation

Projects
None yet
2 participants
@harawata
Copy link
Member

harawata commented Nov 16, 2017

Instead of reading the entire file into String, this version reads a file line by line.

This patch slightly changes the behavior for simplicity.

  1. Version ≤ 3.3.1 allowed any character before @UNDO. 3.3.2 will allow only spaces and tabs. For example, the following undo marker was valid in ≤3.3.1, but will be invalid in 3.3.2.

    -- // some comment @UNDO
    
  2. Version ≤3.3.1 removes the first double slash in any comment line. 3.3.2 will do that only in the @UNDO line.

    -- // some comment (original)
    -- some comment (≤3.3.1)
    -- // some comment (3.3.2)
    

Hi @h3adache ,
Is there any reason these changes could cause some problem?

@h3adache

This comment has been minimized.

Copy link
Member

h3adache commented Nov 17, 2017

No I think those are good changes.
There might be people that put comments before the undo but that wasn't documented.
Our documentation says -- //@UNDO as if it was the token. This is already more lenient than that.

@harawata harawata added this to the 3.3.2 milestone Nov 18, 2017

@harawata harawata changed the title MigrationReader revamp Reduce memory usage when reading a large script file. Nov 18, 2017

@harawata harawata self-assigned this Nov 18, 2017

@harawata harawata merged commit 1e74c3d into mybatis:master Nov 18, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@harawata

This comment has been minimized.

Copy link
Member Author

harawata commented Nov 18, 2017

My thoughts exactly! Thank you, @h3adache !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.