Roslyn: SQLite assembly version need to be revised

Created on 5 Jun 2018  路  16Comments  路  Source: dotnet/roslyn

PR #25047 upgraded that version, which caused insertion issues as there are other products inserting it into VS. PR #27484 reverted that update.

If 1.1.5 works, we can just move to that. Otherwise, we need to ensure the appropriate binding redirects are added to VS. either case, we should also ensure we are consuming the shared copy, rather than inserting our own.

cc @tannergooding @jinujoseph

Area-IDE Bug

Most helpful comment

@KirillOsenkov The gains are non-trivial (imo), but the effort required to actually bump it is really high. It needs to be a coordinated effort across VSWindows teams, and it maybe will be planned for a future release.

I'm hoping we can fix other possible gains and rework the sqlite system so we can fix issues in different ways until we get to that.

All 16 comments

A few notes to this PR:

  1. 1.1.5 will not work as it does not have the right APIs.
  2. It impacts time to code time for VSMac (most likely also does for VSWindows).

Can we slot this for 15.9?

cc @KirillOsenkov

@jinujoseph I've added this to the other 2 blocking issues on the VSMac project: https://github.com/dotnet/roslyn/projects/32

Current status: it's easy for us to bump our version, but there's a few other teams that are _also_ using this in VS and not inside of devenv.exe. So there's a bit of coordination required for this to not break ngen.

@KirillOsenkov any idea what post-15.8 branch you're moving to?

I think we're very flexible here (another way of saying "we have no clue"). We've been taking random Roslyn commits in the past, in fact when we ship releases we freeze whatever last version of Roslyn we took. And since we take NuGet packages, we can take them from anywhere, this is orthogonal to what gets inserted into VS.

@KirillOsenkov Can you describe why this is a blocking issue? PR #27484 reverted Roslyn's use of the new SQLite APIs so there should not be any dependency on one of the new releases. I'd like to close this as a duplicate of #23424 (an opportunistic performance improvement).

@Therzok can answer

Oh, he's on vacation until August 18. OK, from what I know, SQLite 1.1.11 has something that improves VSMac performance, but I don't know what that is. @Therzok has explicitly chosen a build of Roslyn for us previously that has the "thing", it was right before Roslyn downgraded.

Unfortunately I don't have any details.

@kirillosenkov The main idea is that memory allocations from the persistent storage appeared to be top hitters for us, in VSMac.

That includes: diagnostics, global search, typing.

This is an easy change that reduced the biggest traces for those services

Has this been bumped across the board? There's quite a few allocations that could be saved simply because of using the new performance overloads.

We're moving this to backlog based on other recent improvements, but it may get bumped up again in the future.

@Therzok

@KirillOsenkov yep, I know, the current gains from bumping SQLite are non-trivial, and the cruft of the allocations have been fixed via different PRs.

You mean gains are trivial or bumping is non-trivial work? If gains are non-trivial then we should bump soon, no? ;)

@KirillOsenkov The gains are non-trivial (imo), but the effort required to actually bump it is really high. It needs to be a coordinated effort across VSWindows teams, and it maybe will be planned for a future release.

I'm hoping we can fix other possible gains and rework the sqlite system so we can fix issues in different ways until we get to that.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

MadsTorgersen picture MadsTorgersen  路  3Comments

codingonHP picture codingonHP  路  3Comments

JesperTreetop picture JesperTreetop  路  3Comments

DavidArno picture DavidArno  路  3Comments

marler8997 picture marler8997  路  3Comments