Skip to content

Change the Reader and Writer of JSONArray and JSONObject#56

Closed
kraity wants to merge 1 commit intoalibaba:mainfrom
kraity:main
Closed

Change the Reader and Writer of JSONArray and JSONObject#56
kraity wants to merge 1 commit intoalibaba:mainfrom
kraity:main

Conversation

@kraity
Copy link
Copy Markdown
Collaborator

@kraity kraity commented Apr 23, 2022

目前 getObjectReader 和 getObjectWriter 方法中创建实例时没有加锁
获取 Reader 和 Writer 时候可能会出现线程不安全而重复创建
考虑到 JSONArray and JSONObject 的 Reader 和 Writer 使用频率不低, 将其更改为饿汉模式

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 23, 2022 •
edited
Loading

Codecov Report

Merging #56 (4b9225f) into main (c292625) will increase coverage by 0.09%.
The diff coverage is 85.71%.

@@             Coverage Diff              @@
##               main      #56      +/-   ##
============================================
+ Coverage     64.76%   64.86%   +0.09%     
- Complexity     7996     8041      +45     
============================================
  Files           414      420       +6     
  Lines         45284    45402     +118     
  Branches       9144     9156      +12     
============================================
+ Hits          29330    29448     +118     
+ Misses        12401    12397       -4     
- Partials       3553     3557       +4
Impacted Files Coverage Δ
...ava/com/alibaba/fastjson2/reader/ObjectReader.java 57.51% <50.00%> (-0.10%) ⬇️
...ava/com/alibaba/fastjson2/writer/ObjectWriter.java 62.12% <50.00%> (-0.38%) ⬇️
...src/main/java/com/alibaba/fastjson2/JSONArray.java 93.67% <100.00%> (+0.23%) ⬆️
...rc/main/java/com/alibaba/fastjson2/JSONObject.java 84.85% <100.00%> (-0.16%) ⬇️
...rc/main/java/com/alibaba/fastjson2/JSONReader.java 73.10% <0.00%> (-0.31%) ⬇️
...ibaba/fastjson2/writer/ObjectWriterCreatorASM.java 96.61% <0.00%> (-0.05%) ⬇️
...ain/java/com/alibaba/fastjson2/util/BeanUtils.java 67.86% <0.00%> (ø)
...in/java/com/alibaba/fastjson2/codec/FieldInfo.java 100.00% <0.00%> (ø)
...tjson2/reader/FieldReaderAtomicReferenceField.java 68.75% <0.00%> (ø)
...ader/FieldReaderAtomicReferenceMethodReadOnly.java 66.66% <0.00%> (ø)
... and 19 more

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 c292625...4b9225f. Read the comment docs.

@wenshao
Copy link
Copy Markdown
Member

wenshao commented Apr 23, 2022

ObjectReader提供getInstance方法，我担心ObjectReader的实现者会误用，可以用ObjectReaders.of代替。ObjectWriter情况也类似

@kraity
Copy link
Copy Markdown
Collaborator Author

kraity commented Apr 23, 2022

ObjectReader提供getInstance方法，我担心ObjectReader的实现者会误用，可以用ObjectReaders.of代替。ObjectWriter情况也类似

我刚刚测试 ObjectReaders.of 有几个testcase 抛错, ObjectWriters.objectWriter 有80+ testcase 抛错

我先去审查一下

@VictorZeng
Copy link
Copy Markdown
Collaborator

VictorZeng commented Apr 23, 2022

个人感觉不适用单例模式，主要原因如下：

  • 绕过了getObjectReader 和 getObjectWriter 对于Feature.FieldBased的检查
  • 线程问题是否应该考虑采用 AtomicReference.compareAndSet来实现

@kraity
Copy link
Copy Markdown
Collaborator Author

kraity commented Apr 23, 2022

个人感觉不适用单例模式，主要原因如下：

  • 绕过了getObjectReader 和 getObjectWriter 对于Feature.FieldBased的检查
  • 线程问题是否应该考虑采用 AtomicReference.compareAndSet来实现

审查时候发现JSONArray and JSONObject 的 get 方法里, 在之前获取w/r的模式与单例模式相似但非线程安全, 而JSONReader.of(str) JSONWriter.of() 未指定Feature.FieldBased, 所以才想法改用单例

@kraity kraity closed this Apr 23, 2022
@DarkAdventurerr DarkAdventurerr mentioned this pull request Feb 20, 2024
Closed
@hmy361 hmy361 mentioned this pull request Jul 15, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kraity @codecov-commenter @wenshao @VictorZeng