envd icon indicating copy to clipboard operation
envd copied to clipboard

Published 20 hours ago • verified publisher icon tensorchord

WIP chore: optimized the cache mechanism

Open lalawuu opened this issue 3 years ago • 8 comments
trafficstars

This PR is to solve the followings problems:

  • More and more deps files such as requirements.txt
  • Include syntax that import other files

Signed-off-by: nullday [email protected]

lalawuu avatar Sep 14 '22 06:09 lalawuu

Thanks! Feel free to ping me if it is ready to review.

gaocegege avatar Sep 14 '22 06:09 gaocegege

  1. Use the hash of ir.Graph rather than the starlark bytecode hash to distinguish a manifest because of include syntax that will import other envd manifest and the graph is always be the right person.
  2. Search all fields in ir.Graph with keywords like file and path. It is a little tricky because I don't want to define a hook and add it into every compileFunc in lang pkg. It is also a little dangerous If other contributors do not know this convention. So I need your suggestions @gaocegege @zwpaper

lalawuu avatar Sep 14 '22 08:09 lalawuu

Search all fields in ir.Graph with keywords like file and path. It is a little tricky because I don't want to define a hook and add it into every compileFunc in lang pkg. It is also a little dangerous If other contributors do not know this convention. So I need your suggestions @gaocegege @zwpaper

I think we should document it in the code. Maybe add the comments in DefaultGraph.

gaocegege avatar Sep 14 '22 08:09 gaocegege

/cc @VoVAllen @kemingy

gaocegege avatar Sep 14 '22 08:09 gaocegege

Does this mean any field name inside DefaultGraph, with "File"/"Wheel"/"Path" will be considered as dependency and invalidate caches when they change?

VoVAllen avatar Sep 14 '22 09:09 VoVAllen

Does this mean any field name inside DefaultGraph, with "File"/"Wheel"/"Path" will be considered as dependency and invalidate caches when they change?

Bingo!

lalawuu avatar Sep 14 '22 09:09 lalawuu

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aseaday

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

muniu-bot[bot] avatar Sep 20 '22 05:09 muniu-bot[bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aseaday

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

muniu-bot[bot] avatar Sep 20 '22 05:09 muniu-bot[bot]

Maybe we should always rebuild if there is a file from HTTP get.

gaocegege avatar Oct 21 '22 01:10 gaocegege

Maybe we should always rebuild if there is a file from HTTP get.

LGTM

lalawuu avatar Oct 27 '22 03:10 lalawuu

Is this ready to review?

gaocegege avatar Nov 04 '22 06:11 gaocegege

@gaocegege @VoVAllen PTAL

lalawuu avatar Nov 14 '22 05:11 lalawuu

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aseaday, gaocegege, kemingy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • ~~OWNERS~~ [aseaday,gaocegege,kemingy]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

muniu-bot[bot] avatar Nov 14 '22 05:11 muniu-bot[bot]