Skip to content

Replace FlatBuffers fork with upstream 2.0.5 #349

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Dec 29, 2021
Merged

Replace FlatBuffers fork with upstream 2.0.5 #349

merged 4 commits into from
Dec 29, 2021

Conversation

vaind
Copy link
Contributor

@vaind vaind commented Dec 11, 2021 •
edited
Loading

TODO:

  • verify performance in the flutter benchmark (first try seems to show a slight degradation on readAll

@vaind vaind changed the title Replace FlatBuffers fork with upstream 2.0.5 WIP: Replace FlatBuffers fork with upstream 2.0.5 Dec 12, 2021
@vaind
Copy link
Contributor Author

vaind commented Dec 18, 2021 •
edited
Loading

Rough benchmark results

confirms that readAll is slower after this PR.

main @ 6d3bc38

dependency_overrides:
  objectbox:
    git:
      url: [email protected]:objectbox/objectbox-dart.git
      path: objectbox
  objectbox_flutter_libs:
    git:
      url: [email protected]:objectbox/objectbox-dart.git
      path: flutter_libs
  objectbox_generator:
    git:
      url: [email protected]:objectbox/objectbox-dart.git
      path: generator

image image

this branch (FB 2.0.5)

image image

@vaind vaind force-pushed the refactor/fb-2-update branch from 7350eb5 to 5bc9c78 Compare December 18, 2021 08:56
@vaind
Copy link
Contributor Author

vaind commented Dec 18, 2021

After a fix to pass asciOptimization: true to fb.StringReader() (was true by default in the fork):

image

@vaind vaind changed the title WIP: Replace FlatBuffers fork with upstream 2.0.5 Replace FlatBuffers fork with upstream 2.0.5 Dec 18, 2021
@vaind
Copy link
Contributor Author

vaind commented Dec 19, 2021

Good to be merged from my POV, the outstanding failures in CI (flutter beta) are unrelated to this PR

greenrobot-team
greenrobot-team reviewed Dec 20, 2021
View reviewed changes
Copy link
Member

@greenrobot-team greenrobot-team left a comment

Choose a reason for hiding this comment

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

LGTM, just a question about added TODOs.

@vaind vaind mentioned this pull request Dec 29, 2021
Closed
@vaind vaind merged commit 4ec984a into main Dec 29, 2021
@vaind vaind deleted the refactor/fb-2-update branch December 29, 2021 09:35
@greenrobot-team greenrobot-team added this to the 1.4 milestone Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greenrobot-team greenrobot-team greenrobot-team left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.4.0
Development

Successfully merging this pull request may close these issues.
2 participants
@vaind @greenrobot-team