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

Avoid StackOverflowError while reading a VCF with many lines of meta #252

Merged
merged 2 commits into from Dec 23, 2021

Conversation

k-kom
Copy link
Contributor

@k-kom k-kom commented Dec 23, 2021

With current implementation, many lines of meta (say 4k lines of contig definition) causes StackOverflowError like below.

 :cause nil
 :via
 [{:type java.lang.StackOverflowError
   :message nil
   :at [clojure.lang.PersistentHashMap$BitmapIndexedNode ensureEditable "PersistentHashMap.java" 812]}]
 :trace
 [[clojure.lang.PersistentHashMap$BitmapIndexedNode ensureEditable "PersistentHashMap.java" 812]
  [clojure.lang.PersistentHashMap$BitmapIndexedNode assoc "PersistentHashMap.java" 894]
  [clojure.lang.PersistentHashMap$TransientHashMap doAssoc "PersistentHashMap.java" 327]
  [clojure.lang.ATransientMap assoc "ATransientMap.java" 64]
  [clojure.lang.PersistentHashMap create "PersistentHashMap.java" 78]
  [clojure.core$hash_map invokeStatic "core.clj" 389]
  [clojure.core$hash_map doInvoke "core.clj" 381]
  [clojure.lang.RestFn applyTo "RestFn.java" 137]
  [clojure.core$apply invokeStatic "core.clj" 667]
  [clojure.core$apply invoke "core.clj" 662]
  [cljam.io.vcf.reader$parse_data_line invokeStatic "reader.clj" 173]
  [cljam.io.vcf.reader$parse_data_line invoke "reader.clj" 154]
  [cljam.io.vcf.reader$read_data_lines invokeStatic "reader.clj" 181]
  [cljam.io.vcf.reader$read_data_lines invoke "reader.clj" 175]
  [cljam.io.vcf.reader$read_data_lines invokeStatic "reader.clj" 183]

;; continues recursive call's stack trace

I fixed the problem by explicitly calling recur to utilize tail call optimization.

…meta

With current implementation, many lines of meta (say 4k lines of contig definition) causes StackOverflowError like below.

```
 :cause nil
 :via
 [{:type java.lang.StackOverflowError
   :message nil
   :at [clojure.lang.PersistentHashMap$BitmapIndexedNode ensureEditable "PersistentHashMap.java" 812]}]
 :trace
 [[clojure.lang.PersistentHashMap$BitmapIndexedNode ensureEditable "PersistentHashMap.java" 812]
  [clojure.lang.PersistentHashMap$BitmapIndexedNode assoc "PersistentHashMap.java" 894]
  [clojure.lang.PersistentHashMap$TransientHashMap doAssoc "PersistentHashMap.java" 327]
  [clojure.lang.ATransientMap assoc "ATransientMap.java" 64]
  [clojure.lang.PersistentHashMap create "PersistentHashMap.java" 78]
  [clojure.core$hash_map invokeStatic "core.clj" 389]
  [clojure.core$hash_map doInvoke "core.clj" 381]
  [clojure.lang.RestFn applyTo "RestFn.java" 137]
  [clojure.core$apply invokeStatic "core.clj" 667]
  [clojure.core$apply invoke "core.clj" 662]
  [cljam.io.vcf.reader$parse_data_line invokeStatic "reader.clj" 173]
  [cljam.io.vcf.reader$parse_data_line invoke "reader.clj" 154]
  [cljam.io.vcf.reader$read_data_lines invokeStatic "reader.clj" 181]
  [cljam.io.vcf.reader$read_data_lines invoke "reader.clj" 175]
  [cljam.io.vcf.reader$read_data_lines invokeStatic "reader.clj" 183]

;; continues recursive call's stack trace
```

I fixed the problem explicitly call `recur` to utlize tail call optimization.
@k-kom k-kom self-assigned this Dec 23, 2021
@k-kom k-kom requested review from alumi and a team as code owners December 23, 2021 07:42
@k-kom k-kom requested review from athos and removed request for a team December 23, 2021 07:42
@codecov
Copy link

codecov bot commented Dec 23, 2021

Codecov Report

Merging #252 (ce4242b) into master (23392cb) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #252   +/-   ##
=======================================
  Coverage   88.70%   88.70%           
=======================================
  Files          76       76           
  Lines        6305     6305           
  Branches      438      438           
=======================================
  Hits         5593     5593           
  Misses        274      274           
  Partials      438      438           
Impacted Files Coverage Δ
src/cljam/io/vcf/reader.clj 87.73% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23392cb...ce4242b. Read the comment docs.

Copy link
Member

@athos athos left a comment

Choose a reason for hiding this comment

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

Thank you for the fix! LGTM 👍

Copy link
Member

@alumi alumi left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! LGTM 👍

@alumi alumi merged commit 7e7ba7f into master Dec 23, 2021
@alumi alumi deleted the fix/stack-overflow-with-many-lines-of-meta branch December 23, 2021 08:50
@alumi alumi added the bug label Dec 23, 2021
@alumi alumi mentioned this pull request Dec 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants