Skip to content

Commit

Permalink
Retire the "attributes" database mechanism from Glass
Browse files Browse the repository at this point in the history
Summary:
About 3 years ago when we were shipping codehub navigation for the first time, one of the blockers was showing gCPU data from Xenon on hover cards.
This was a feature in Diffusion that we had to match to allow CodeHub Browser to ship.

The problem was the xenon data only knew Hack "class::method" qnames. While in Glean we had entities. We had no way at the time to stack the www.perf.hack data over the Hack db, so instead Glass did a client-side "join" on document symbols.

This meant opening an additional attributes db, then doing a bulk fetch of any extra metadata by qname.
The safety was enforced by a type level mechanism that associates a key in the attribute db with an entity in the base db.

I think now days we would add this extra metadata at indexing time (ideally), where the Hack indexer would grab the daily xenon data as a final step in indexnig.
Alternativley, you'd stack the perf data over the base Hack db and join on entities.

We don't need this stuff anymore since the www.perf.hack db has gone away and no one cared.
We _should_ revisit how to store Xenon data properly.

Reviewed By: simonmar

Differential Revision: D56339981

fbshipit-source-id: da55c060cc3112cf3784f7b34ebc390f8590dc66
  • Loading branch information
donsbot authored and facebook-github-bot committed Apr 19, 2024
1 parent cdecec4 commit 8bcc177
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 144 deletions.
22 changes: 1 addition & 21 deletions glean/glass/Glean/Glass/Base.hs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ module Glean.Glass.Base
( GleanDBName(..)
, GleanPath(..)
, SymbolRepoPath(..)
, GleanDBAttrName(..)
, RepoMapping(..)
) where

Expand All @@ -19,9 +18,7 @@ import qualified Data.Map as Map
import Data.String
import Data.Text (Text)

import Glean.Glass.Attributes.Class as Attributes
import qualified Glean.Glass.Types as Glass
import Glean.Glass.Utils

-- | Type of glean dbs
newtype GleanDBName = GleanDBName { unGleanDBName :: Text }
Expand All @@ -45,18 +42,7 @@ data SymbolRepoPath = SymbolRepoPath
}
deriving (Show, Eq)

-- | A little existential type key for attributes
data GleanDBAttrName =
forall attr .
( QueryType (Attributes.AttrRep attr)
, Attributes.ToAttributes attr)
=>
GleanDBAttrName {
gleanAttrDBName :: GleanDBName,
attributeKey :: attr
}

data RepoMapping = RepoMapping
newtype RepoMapping = RepoMapping
{ gleanIndices :: Map.Map Glass.RepoName [(GleanDBName, Glass.Language)]
-- ^ Glean indexes and the language they index. This should be in a config
--
Expand All @@ -68,10 +54,4 @@ data RepoMapping = RepoMapping
--
-- If you add/remove a db, consider if it needs to be present in
-- gleanRequiredIndices as well

, gleanAttrIndices :: Map.Map GleanDBName [GleanDBAttrName]
-- ^ Map of language/source db pairs to attr db names & attribute key types
--
-- This pairs attribute Glean dbs with a key type to index the ToAttribute
-- class, that in turns knowns how to query and marshal the attributes
}
105 changes: 11 additions & 94 deletions glean/glass/Glean/Glass/Handler.hs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ import Data.Either.Extra (eitherToMaybe)
import Data.Default (def)
import Data.List as List ( sortOn )
import Data.List.Extra ( nubOrd, nubOrdOn, groupOn, groupSortOn )
import Data.List.NonEmpty (NonEmpty(..), toList, nonEmpty)
import Data.List.NonEmpty (NonEmpty(..), nonEmpty)
import qualified Data.List.NonEmpty as NonEmpty
import Data.Maybe ( mapMaybe, catMaybes, fromMaybe, listToMaybe )
import Data.Ord ( comparing )
Expand Down Expand Up @@ -125,7 +125,6 @@ import Glean.Glass.Utils
import Glean.Glass.Attributes.SymbolKind
( symbolKindToSymbolKind, symbolKindFromSymbolKind )
import qualified Glean.Glass.SearchRelated as Search
import Glean.Glass.SourceControl
import Glean.Glass.SearchRelated
( RelatedLocatedEntities(childRL, parentRL),
Recursive(NotRecursive) )
Expand All @@ -143,8 +142,7 @@ import Glean.Glass.Tracing (GlassTracer, traceSpan)
runRepoFile
:: (LogResult t)
=> Text
-> (Some SourceControl
-> RepoMapping
-> (RepoMapping
-> GleanDBInfo
-> DocumentSymbolsRequest
-> RequestOptions
Expand All @@ -158,7 +156,7 @@ runRepoFile
-> IO t
runRepoFile sym fn env@Glass.Env{..} req opts =
withRepoFile sym env opts req repo file $ \gleanDBs dbInfo mlang ->
fn sourceControl (Glass.repoMapping env) dbInfo req opts
fn (Glass.repoMapping env) dbInfo req opts
GleanBackend{..}
snapshotBackend
mlang
Expand Down Expand Up @@ -776,7 +774,7 @@ searchBySymbolId [email protected]{..} symbolPrefix opts = do
data FileReference =
FileReference {
_repoName :: !RepoName,
theGleanPath :: !GleanPath
_theGleanPath :: !GleanPath
}

toFileReference :: RepoName -> Path -> FileReference
Expand Down Expand Up @@ -889,21 +887,18 @@ withEntity f scsrepo lang toks = do
fetchSymbolsAndAttributesGlean
:: Glean.Backend b
=> GlassTracer
-> Some SourceControl
-> RepoMapping
-> GleanDBInfo
-> DocumentSymbolsRequest
-> RequestOptions
-> GleanBackend b
-> Maybe Language
-> IO ((DocumentSymbolListXResult, QueryEachRepoLog), Maybe ErrorLogger)
fetchSymbolsAndAttributesGlean tracer scm repoMapping dbInfo req opts be mlang = do
fetchSymbolsAndAttributesGlean tracer repoMapping dbInfo req opts be mlang = do
(res1, gLogs, elogs) <- traceSpan tracer "fetchDocumentSymbols" $
fetchDocumentSymbols file mlimit
specificRev includeRefs includeXlangRefs be mlang repoMapping dbInfo
res2 <- traceSpan tracer "addDynamicAttributes" $
addDynamicAttributes scm repoMapping dbInfo repo opts
file mlimit be res1
let res2 = toDocumentSymbolResult res1
return ((res2, gLogs), elogs)
where
repo = documentSymbolsRequest_repository req
Expand All @@ -925,7 +920,6 @@ fetchSymbolsAndAttributesGlean tracer scm repoMapping dbInfo req opts be mlang =
fetchSymbolsAndAttributes
:: (Glean.Backend b, SnapshotBackend snapshotBackend)
=> GlassTracer
-> Some SourceControl
-> RepoMapping
-> GleanDBInfo
-> DocumentSymbolsRequest
Expand All @@ -935,7 +929,7 @@ fetchSymbolsAndAttributes
-> Maybe Language
-> IO ((DocumentSymbolListXResult, SnapshotStatus, QueryEachRepoLog)
, Maybe ErrorLogger)
fetchSymbolsAndAttributes tracer scm repoMapping dbInfo req opts be
fetchSymbolsAndAttributes tracer repoMapping dbInfo req opts be
snapshotbe mlang = do
res <- case mrevision of
Just revision -> do
Expand Down Expand Up @@ -973,8 +967,7 @@ fetchSymbolsAndAttributes tracer scm repoMapping dbInfo req opts be
where
addStatus st ((res, gleanLog), mlogger) = ((res, st, gleanLog), mlogger)
getFromGlean =
fetchSymbolsAndAttributesGlean
tracer scm repoMapping dbInfo req opts be mlang
fetchSymbolsAndAttributesGlean tracer repoMapping dbInfo req opts be mlang
file = documentSymbolsRequest_filepath req
repo = documentSymbolsRequest_repository req
mrevision = requestOptions_revision opts
Expand Down Expand Up @@ -1125,37 +1118,6 @@ toDocumentSymbolResult DocumentSymbols{..} = DocumentSymbolListXResult{..}
documentSymbolListXResult_digest = digest
documentSymbolListXResult_referenced_file_digests = xref_digests

--
-- | Check if this db / lang pair has additional dynamic attributes
-- and add them if so
--
addDynamicAttributes
:: Glean.Backend b
=> Some SourceControl
-> RepoMapping
-> GleanDBInfo
-> RepoName
-> RequestOptions
-> FileReference
-> Maybe Int
-> GleanBackend b
-> DocumentSymbols
-> IO DocumentSymbolListXResult
addDynamicAttributes scm repoMapping dbInfo repo opts repofile
mlimit be syms = do
-- combine additional dynamic attributes
mattrs <- getSymbolAttributes scm repoMapping dbInfo repo opts
repofile mlimit be
return $ extend mattrs syms
where
extend [] syms = toDocumentSymbolResult syms
extend ((GleanDBAttrName _ attrKey, attrMap) : xs) syms =
let keyFn = Attributes.fromSymbolId attrKey
(refs',defs') = Attributes.extendAttributes keyFn attrMap
(refs syms)
(defs syms)
in extend xs $ syms { refs = refs' , defs = defs' }

type Defns = [(Code.Location,Code.Entity)]

-- | Which fileEntityLocations and fileEntityXRefLocation implementations to use
Expand Down Expand Up @@ -1187,7 +1149,6 @@ documentSymbolsForLanguage mlimit _ includeRefs _ fileId = do
fetchDocumentSymbolIndex
:: (Glean.Backend b, SnapshotBackend snapshotBackend)
=> GlassTracer
-> Some SourceControl
-> RepoMapping
-> GleanDBInfo
-> DocumentSymbolsRequest
Expand All @@ -1197,10 +1158,11 @@ fetchDocumentSymbolIndex
-> Maybe Language
-> IO ((DocumentSymbolIndex, SnapshotStatus, QueryEachRepoLog),
Maybe ErrorLogger)
fetchDocumentSymbolIndex tracer scm repoMapping latest req opts be
fetchDocumentSymbolIndex tracer repoMapping latest req opts be
snapshotbe mlang = do
((DocumentSymbolListXResult{..}, status, gleanDataLog), merr1) <-
fetchSymbolsAndAttributes tracer scm repoMapping latest req opts be snapshotbe mlang
fetchSymbolsAndAttributes
tracer repoMapping latest req opts be snapshotbe mlang

-- refs defs revision truncated digest = result
let lineIndex = toSymbolIndex documentSymbolListXResult_references
Expand All @@ -1219,32 +1181,6 @@ fetchDocumentSymbolIndex tracer scm repoMapping latest req opts be
}
return ((idxResult, status, gleanDataLog), merr1)

-- Work out if we have extra attribute dbs and then run the queries
getSymbolAttributes
:: Glean.Backend b
=> Some SourceControl
-> RepoMapping
-> GleanDBInfo
-> RepoName
-> RequestOptions
-> FileReference
-> Maybe Int
-> GleanBackend b
-> IO
[(GleanDBAttrName, Map.Map Attributes.SymbolIdentifier Attributes)]
getSymbolAttributes scm repoMapping dbInfo repo opts repofile mlimit
be@GleanBackend{..} = do
mAttrDBs <- forM (map fst $ toList gleanDBs) $
getLatestAttrDB tracer scm repoMapping dbInfo repo opts
attrs <- backendRunHaxl be $ do
forM (catMaybes mAttrDBs) $
\(attrDB, attr@(GleanDBAttrName _ attrKey){- existential key -}) ->
withRepo attrDB $ do
(attrMap,_merr2) <- genericFetchFileAttributes attrKey
(theGleanPath repofile) mlimit
return $ Just (attr, attrMap)
return $ catMaybes attrs

-- | Same repo generic attributes
documentSymbolKinds
:: Maybe Int
Expand All @@ -1262,25 +1198,6 @@ documentSymbolKinds _mlimit (Just Language_Cpp) _fileId =
documentSymbolKinds mlimit _ fileId =
searchFileAttributes Attributes.SymbolKindAttr mlimit fileId

-- | External (non-local db) Attributes of symbols. Just Hack only for now
genericFetchFileAttributes
:: (QueryType (Attributes.AttrRep key)
, Attributes.ToAttributes key)
=> key
-> GleanPath
-> Maybe Int
-> RepoHaxl u w
(Map.Map Attributes.SymbolIdentifier Attributes, Maybe ErrorLogger)

genericFetchFileAttributes key path mlimit = do
efile <- getFile path
repo <- Glean.haxlRepo
case efile of
Left err ->
return (mempty, Just (logError err <> logError repo))
Right fileId -> do
searchFileAttributes key mlimit (Glean.getId fileId)

searchFileAttributes
:: (QueryType (Attributes.AttrRep key), Attributes.ToAttributes key)
=> key
Expand Down
21 changes: 0 additions & 21 deletions glean/glass/Glean/Glass/Handler/Utils.hs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ module Glean.Glass.Handler.Utils (

-- * deprecated handler wrappers
withGleanDBs,
getLatestAttrDB,

-- * Utils for building handlers
GleanBackend(..),
Expand Down Expand Up @@ -124,26 +123,6 @@ dbChooser repo opts =
nearest = requestOptions_feature_flags opts >>= featureFlags_nearest_revision
exact = requestOptions_exact_revision opts

-- | Get glean db for an attribute type
getLatestAttrDB
:: GlassTracer
-> Some SourceControl
-> RepoMapping
-> GleanDBInfo
-> RepoName
-> RequestOptions
-> GleanDBName
-> IO (Maybe (Glean.Repo, GleanDBAttrName))
getLatestAttrDB tracer scm repoMapping dbInfo repo opts gleanDBName =
case firstAttrDB repoMapping gleanDBName of
Nothing -> return Nothing
Just attrDBName -> do
dbs <- chooseGleanDBs tracer scm dbInfo (dbChooser repo opts)
[gleanAttrDBName attrDBName]
return $ case dbs of
[] -> Nothing
db:_ -> Just (snd db, attrDBName)

withGleanDBs
:: (LogError a, LogRequest a, LogResult b)
=> Text
Expand Down
8 changes: 0 additions & 8 deletions glean/glass/Glean/Glass/Repos.hs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,13 @@ module Glean.Glass.Repos
-- * Types
Language(..)
, GleanDBName(..)
, GleanDBAttrName(..)
, GleanDBInfo(..)
, ScmRevisions
, ScmRevisionInfo(..)

-- * Mappings
, fromSCSRepo
, filetype
, firstAttrDB

-- * Operation on a pool of latest repos
, withLatestRepos
Expand Down Expand Up @@ -112,12 +110,6 @@ toRepoName RepoMapping{..} repo =
where
repoName = RepoName repo

-- | Additional metadata about files and methods in attribute dbs
firstAttrDB :: RepoMapping -> GleanDBName -> Maybe GleanDBAttrName
firstAttrDB RepoMapping{..} dbName
| Just (db:_) <- Map.lookup dbName gleanAttrIndices = Just db
| otherwise = Nothing

-- | Expand generic string search request parameters into a set of candidate
-- GleanDBs, grouped by logical SCM repo or corpus.
--
Expand Down

0 comments on commit 8bcc177

Please sign in to comment.